From c522addab543cf01b5ac1116cb23f0ffda118838 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 2 Apr 2025 16:57:55 -0700 Subject: [PATCH] [RISCV] Check S0 register list check for qc.cm.pushfp to after we parsed the whole register list. In my mind, this is more of a semantic check. I've also changed the diagnostic location to point at the register list start instead of the closing brace, or whatever character might be there instead of a brace if its malformed. --- .../Target/RISCV/AsmParser/RISCVAsmParser.cpp | 20 +++++-------------- llvm/test/MC/RISCV/rv32xqccmp-invalid.s | 2 +- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp index 7837504751694..ed890104d488d 100644 --- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp +++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp @@ -2588,20 +2588,8 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands, return Error(getLoc(), "register list must start from 'ra' or 'x1'"); getLexer().Lex(); - bool SeenComma = parseOptionalToken(AsmToken::Comma); - - // There are two choices here: - // - `s0` is not required (usual case), so only try to parse `s0` if there is - // a comma - // - `s0` is required (qc.cm.pushfp), and so we must see the comma between - // `ra` and `s0` and must always try to parse `s0`, below - if (MustIncludeS0 && !SeenComma) { - Error(getLoc(), "register list must include 's0' or 'x8'"); - return ParseStatus::Failure; - } - // parse case like ,s0 (knowing the comma must be there if required) - if (SeenComma) { + if (parseOptionalToken(AsmToken::Comma)) { if (getLexer().isNot(AsmToken::Identifier)) return Error(getLoc(), "invalid register"); StringRef RegName = getLexer().getTok().getIdentifier(); @@ -2664,8 +2652,10 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands, auto Encode = RISCVZC::encodeRlist(RegEnd, IsRVE); assert(Encode != RISCVZC::INVALID_RLIST); - if (MustIncludeS0) - assert(Encode != RISCVZC::RA); + + if (MustIncludeS0 && Encode == RISCVZC::RA) + return Error(S, "register list must include 's0' or 'x8'"); + Operands.push_back(RISCVOperand::createRlist(Encode, S)); return ParseStatus::Success; diff --git a/llvm/test/MC/RISCV/rv32xqccmp-invalid.s b/llvm/test/MC/RISCV/rv32xqccmp-invalid.s index e43f86cbb84ee..5bfc2e3498bef 100644 --- a/llvm/test/MC/RISCV/rv32xqccmp-invalid.s +++ b/llvm/test/MC/RISCV/rv32xqccmp-invalid.s @@ -34,6 +34,6 @@ qc.cm.pushfp {ra, s0}, -12 # CHECK-ERROR: :[[@LINE+1]]:24: error: stack adjustment for register list must be a multiple of 16 bytes in the range [16, 64] qc.cm.pop {ra, s0-s1}, -40 -# CHECK-ERROR: :[[@LINE+1]]:17: error: register list must include 's0' or 'x8' +# CHECK-ERROR: :[[@LINE+1]]:14: error: register list must include 's0' or 'x8' qc.cm.pushfp {ra}, -16