[llvm] [RISCV] Refactor register list parsing and improve error messages. (PR #134938)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 8 15:41:42 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mc
Author: Craig Topper (topperc)
<details>
<summary>Changes</summary>
Structure the code into a loop that parses a series of ranges and gives an error when there are too many ranges.
Give errors when ABI and non-ABI names are mixed.
Properly diagnose 'x1-` starting a list.
Use a default bool argument to merge parseRegListCommon and parseRegList.
---
Full diff: https://github.com/llvm/llvm-project/pull/134938.diff
5 Files Affected:
- (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+72-74)
- (modified) llvm/test/MC/RISCV/rv32xqccmp-invalid.s (+1-1)
- (modified) llvm/test/MC/RISCV/rv32zcmp-invalid.s (+14-11)
- (modified) llvm/test/MC/RISCV/rv64xqccmp-invalid.s (+1-1)
- (modified) llvm/test/MC/RISCV/rv64zcmp-invalid.s (+54-12)
``````````diff
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index dba78fef0bad8..a1b3a3f299829 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -215,13 +215,10 @@ class RISCVAsmParser : public MCTargetAsmParser {
ParseStatus parseGPRPair(OperandVector &Operands, bool IsRV64Inst);
ParseStatus parseFRMArg(OperandVector &Operands);
ParseStatus parseFenceArg(OperandVector &Operands);
- ParseStatus parseRegList(OperandVector &Operands) {
- return parseRegListCommon(Operands, /*MustIncludeS0=*/false);
- }
+ ParseStatus parseRegList(OperandVector &Operands, bool MustIncludeS0 = false);
ParseStatus parseRegListS0(OperandVector &Operands) {
- return parseRegListCommon(Operands, /*MustIncludeS0=*/true);
+ return parseRegList(Operands, /*MustIncludeS0=*/true);
}
- ParseStatus parseRegListCommon(OperandVector &Operands, bool MustIncludeS0);
ParseStatus parseRegReg(OperandVector &Operands);
ParseStatus parseRetval(OperandVector &Operands);
@@ -2567,96 +2564,97 @@ ParseStatus RISCVAsmParser::parseRegReg(OperandVector &Operands) {
return ParseStatus::Success;
}
-ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
- bool MustIncludeS0) {
- // RegList: {ra [, s0[-sN]]}
- // XRegList: {x1 [, x8[-x9][, x18[-xN]]]}
-
- // When MustIncludeS0 = true (not the default) (used for `qc.cm.pushfp`) which
- // must include `fp`/`s0` in the list:
- // RegList: {ra, s0[-sN]}
- // XRegList: {x1, x8[-x9][, x18[-xN]]}
+// RegList: {ra [, s0[-sN]]}
+// XRegList: {x1 [, x8[-x9][, x18[-xN]]]}
+// When MustIncludeS0 = true (not the default) (used for `qc.cm.pushfp`) which
+// must include `fp`/`s0` in the list:
+// RegList: {ra, s0[-sN]}
+// XRegList: {x1, x8[-x9][, x18[-xN]]}
+ParseStatus RISCVAsmParser::parseRegList(OperandVector &Operands,
+ bool MustIncludeS0) {
if (getTok().isNot(AsmToken::LCurly))
return ParseStatus::NoMatch;
SMLoc S = getLoc();
+
Lex();
- bool IsRVE = isRVE();
+ bool UsesXRegs;
+ MCRegister RegEnd;
+ do {
+ if (getTok().isNot(AsmToken::Identifier) ||
+ (RegEnd == RISCV::X18 && isRVE()))
+ return Error(getLoc(), "invalid register");
- if (getLexer().isNot(AsmToken::Identifier))
- return Error(getLoc(), "register list must start from 'ra' or 'x1'");
+ StringRef RegName = getTok().getIdentifier();
+ MCRegister Reg = matchRegisterNameHelper(RegName);
+ if (!Reg)
+ return Error(getLoc(), "invalid register");
- StringRef RegName = getLexer().getTok().getIdentifier();
- MCRegister RegEnd = matchRegisterNameHelper(RegName);
- if (RegEnd != RISCV::X1)
- return Error(getLoc(), "register list must start from 'ra' or 'x1'");
- getLexer().Lex();
+ if (!RegEnd) {
+ UsesXRegs = RegName[0] == 'x';
+ if (Reg != RISCV::X1)
+ return Error(getLoc(), "register list must start from 'ra' or 'x1'");
+ } else if (RegEnd == RISCV::X1) {
+ if (Reg != RISCV::X8 || (UsesXRegs != (RegName[0] == 'x')))
+ return Error(getLoc(), Twine("register must be '") +
+ (UsesXRegs ? "x8" : "s0") + "'");
+ } else if (RegEnd == RISCV::X9 && UsesXRegs) {
+ if (Reg != RISCV::X18 || (RegName[0] != 'x'))
+ return Error(getLoc(), "register must be 'x18'");
+ } else {
+ return Error(getLoc(), "too many register ranges");
+ }
- // parse case like ,s0 (knowing the comma must be there if required)
- if (parseOptionalToken(AsmToken::Comma)) {
- if (getLexer().isNot(AsmToken::Identifier))
- return Error(getLoc(), "invalid register");
- StringRef RegName = getLexer().getTok().getIdentifier();
- RegEnd = matchRegisterNameHelper(RegName);
- if (!RegEnd)
- return Error(getLoc(), "invalid register");
- if (RegEnd != RISCV::X8)
- return Error(getLoc(),
- "continuous register list must start from 's0' or 'x8'");
- getLexer().Lex(); // eat reg
+ RegEnd = Reg;
- // parse case like -s1
+ Lex();
+
+ SMLoc MinusLoc = getLoc();
if (parseOptionalToken(AsmToken::Minus)) {
- StringRef EndName = getLexer().getTok().getIdentifier();
- // FIXME: the register mapping and checks of RVE is wrong
- RegEnd = matchRegisterNameHelper(EndName);
- if (!(RegEnd == RISCV::X9 ||
- (RegEnd >= RISCV::X18 && RegEnd <= RISCV::X27)))
- return Error(getLoc(), "invalid register");
- getLexer().Lex();
- }
+ if (RegEnd == RISCV::X1)
+ return Error(MinusLoc, Twine("register '") + (UsesXRegs ? "x1" : "ra") +
+ "' cannot start a multiple register range");
- // parse extra part like ', x18[-x20]' for XRegList
- if (parseOptionalToken(AsmToken::Comma)) {
- if (RegEnd != RISCV::X9)
- return Error(
- getLoc(),
- "first contiguous registers pair of register list must be 'x8-x9'");
+ if (getTok().isNot(AsmToken::Identifier) ||
+ (RegEnd == RISCV::X18 && isRVE()))
+ return Error(getLoc(), "invalid register");
- // parse ', x18' for extra part
- if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
+ StringRef RegName = getTok().getIdentifier();
+ MCRegister Reg = matchRegisterNameHelper(RegName);
+ if (!Reg)
return Error(getLoc(), "invalid register");
- StringRef EndName = getLexer().getTok().getIdentifier();
- RegEnd = MatchRegisterName(EndName);
- if (RegEnd != RISCV::X18)
- return Error(getLoc(),
- "second contiguous registers pair of register list "
- "must start from 'x18'");
- getLexer().Lex();
-
- // parse '-x20' for extra part
- if (parseOptionalToken(AsmToken::Minus)) {
- if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
- return Error(getLoc(), "invalid register");
- EndName = getLexer().getTok().getIdentifier();
- RegEnd = MatchRegisterName(EndName);
- if (!(RegEnd >= RISCV::X19 && RegEnd <= RISCV::X27))
- return Error(getLoc(), "invalid register");
- getLexer().Lex();
- }
+
+ if (RegEnd == RISCV::X8) {
+ if ((Reg != RISCV::X9 &&
+ (UsesXRegs || Reg < RISCV::X18 || Reg > RISCV::X27)) ||
+ (UsesXRegs != (RegName[0] == 'x'))) {
+ if (UsesXRegs)
+ return Error(getLoc(), "register must be 'x9'");
+ return Error(getLoc(), "register must be in the range 's1' to 's11'");
+ }
+ } else if (RegEnd == RISCV::X18) {
+ if (Reg < RISCV::X19 || Reg > RISCV::X27 || (RegName[0] != 'x'))
+ return Error(getLoc(),
+ "register must be in the range 'x19' to 'x27'");
+ } else
+ llvm_unreachable("unexpected register");
+
+ RegEnd = Reg;
+
+ Lex();
}
- }
+ } while (parseOptionalToken(AsmToken::Comma));
- if (parseToken(AsmToken::RCurly, "register list must end with '}'"))
+ if (parseToken(AsmToken::RCurly, "expected ',' or '}'"))
return ParseStatus::Failure;
if (RegEnd == RISCV::X26)
- return Error(S, "invalid register list, {ra, s0-s10} or {x1, x8-x9, "
- "x18-x26} is not supported");
+ return Error(S, "invalid register list, '{ra, s0-s10}' or '{x1, x8-x9, "
+ "x18-x26}' is not supported");
- auto Encode = RISCVZC::encodeRegList(RegEnd, IsRVE);
+ auto Encode = RISCVZC::encodeRegList(RegEnd, isRVE());
assert(Encode != RISCVZC::INVALID_RLIST);
if (MustIncludeS0 && Encode == RISCVZC::RA)
diff --git a/llvm/test/MC/RISCV/rv32xqccmp-invalid.s b/llvm/test/MC/RISCV/rv32xqccmp-invalid.s
index ece3513120392..a955a33dc5a55 100644
--- a/llvm/test/MC/RISCV/rv32xqccmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv32xqccmp-invalid.s
@@ -10,7 +10,7 @@ qc.cm.mvsa01 s0, s0
# CHECK-ERROR: :[[@LINE+1]]:14: error: invalid operand for instruction
qc.cm.mva01s a1, a2
-# CHECK-ERROR: :[[@LINE+1]]:15: error: invalid register list, {ra, s0-s10} or {x1, x8-x9, x18-x26} is not supported
+# CHECK-ERROR: :[[@LINE+1]]:15: error: invalid register list, '{ra, s0-s10}' or '{x1, x8-x9, x18-x26}' is not supported
qc.cm.popretz {ra, s0-s10}, 112
# CHECK-ERROR: :[[@LINE+1]]:28: error: stack adjustment for register list must be a multiple of 16 bytes in the range [16, 64]
diff --git a/llvm/test/MC/RISCV/rv32zcmp-invalid.s b/llvm/test/MC/RISCV/rv32zcmp-invalid.s
index b4261f865fae7..68aa7fc548f61 100644
--- a/llvm/test/MC/RISCV/rv32zcmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv32zcmp-invalid.s
@@ -10,7 +10,7 @@ cm.mvsa01 s0, s0
# CHECK-ERROR: :[[@LINE+1]]:11: error: invalid operand for instruction
cm.mva01s a1, a2
-# CHECK-ERROR: :[[@LINE+1]]:12: error: invalid register list, {ra, s0-s10} or {x1, x8-x9, x18-x26} is not supported
+# CHECK-ERROR: :[[@LINE+1]]:12: error: invalid register list, '{ra, s0-s10}' or '{x1, x8-x9, x18-x26}' is not supported
cm.popretz {ra, s0-s10}, 112
# CHECK-ERROR: :[[@LINE+1]]:25: error: stack adjustment for register list must be a multiple of 16 bytes in the range [16, 64]
@@ -28,23 +28,26 @@ cm.push {ra}, -8
# CHECK-ERROR: :[[@LINE+1]]:9: error: register list must start from 'ra' or 'x1'
cm.pop {s0}, -40
-# CHECK-ERROR: :[[@LINE+1]]:13: error: continuous register list must start from 's0' or 'x8'
+# CHECK-ERROR: :[[@LINE+1]]:13: error: register must be 's0'
cm.pop {ra, t1}, -40
-# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be in the range 's1' to 's11'
cm.pop {ra, s0-t1}, -40
-# CHECK-ERROR: :[[@LINE+1]]:20: error: second contiguous registers pair of register list must start from 'x18'
-cm.pop {ra, x8-x9, x28}, -40
+# CHECK-ERROR: :[[@LINE+1]]:20: error: register must be 'x18'
+cm.pop {x1, x8-x9, x28}, -40
-# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x28}, -40
+# CHECK-ERROR: :[[@LINE+1]]:24: error: register must be in the range 'x19' to 'x27'
+cm.pop {x1, x8-x9, x18-x28}, -40
-# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x17}, -40
+# CHECK-ERROR: :[[@LINE+1]]:24: error: register must be in the range 'x19' to 'x27'
+cm.pop {x1, x8-x9, x18-x17}, -40
-# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
-cm.pop {ra, x8-f8, x18-x17}, -40
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be 'x9'
+cm.pop {x1, x8-f8, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:9: error: operand must be {ra [, s0[-sN]]} or {x1 [, x8[-x9][, x18[-xN]]]}
cm.push x1, -16
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be in the range 's1' to 's11'
+cm.pop {ra, s0-f8}, -40
diff --git a/llvm/test/MC/RISCV/rv64xqccmp-invalid.s b/llvm/test/MC/RISCV/rv64xqccmp-invalid.s
index 953887cc77e47..24ffc6b9fc02a 100644
--- a/llvm/test/MC/RISCV/rv64xqccmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv64xqccmp-invalid.s
@@ -10,7 +10,7 @@ qc.cm.mvsa01 s0, s0
# CHECK-ERROR: :[[@LINE+1]]:14: error: invalid operand for instruction
qc.cm.mva01s a1, a2
-# CHECK-ERROR: :[[@LINE+1]]:15: error: invalid register list, {ra, s0-s10} or {x1, x8-x9, x18-x26} is not supported
+# CHECK-ERROR: :[[@LINE+1]]:15: error: invalid register list, '{ra, s0-s10}' or '{x1, x8-x9, x18-x26}' is not supported
qc.cm.popretz {ra, s0-s10}, 112
# CHECK-ERROR: :[[@LINE+1]]:28: error: stack adjustment for register list must be a multiple of 16 bytes in the range [32, 80]
diff --git a/llvm/test/MC/RISCV/rv64zcmp-invalid.s b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
index c66415cb49b34..e806da81ea6d3 100644
--- a/llvm/test/MC/RISCV/rv64zcmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
@@ -10,7 +10,7 @@ cm.mvsa01 s0, s0
# CHECK-ERROR: :[[@LINE+1]]:11: error: invalid operand for instruction
cm.mva01s a1, a2
-# CHECK-ERROR: :[[@LINE+1]]:12: error: invalid register list, {ra, s0-s10} or {x1, x8-x9, x18-x26} is not supported
+# CHECK-ERROR: :[[@LINE+1]]:12: error: invalid register list, '{ra, s0-s10}' or '{x1, x8-x9, x18-x26}' is not supported
cm.popretz {ra, s0-s10}, 112
# CHECK-ERROR: :[[@LINE+1]]:25: error: stack adjustment for register list must be a multiple of 16 bytes in the range [32, 80]
@@ -31,23 +31,23 @@ cm.pop {ra, s0-s1}, -33
# CHECK-ERROR: :[[@LINE+1]]:9: error: register list must start from 'ra' or 'x1'
cm.pop {s0}, -40
-# CHECK-ERROR: :[[@LINE+1]]:13: error: continuous register list must start from 's0' or 'x8'
+# CHECK-ERROR: :[[@LINE+1]]:13: error: register must be 's0'
cm.pop {ra, t1}, -40
-# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be in the range 's1' to 's11'
cm.pop {ra, s0-t1}, -40
-# CHECK-ERROR: :[[@LINE+1]]:20: error: second contiguous registers pair of register list must start from 'x18'
-cm.pop {ra, x8-x9, x28}, -40
+# CHECK-ERROR: :[[@LINE+1]]:20: error: register must be 'x18'
+cm.pop {x1, x8-x9, x28}, -40
-# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x28}, -40
+# CHECK-ERROR: :[[@LINE+1]]:24: error: register must be in the range 'x19' to 'x27'
+cm.pop {x1, x8-x9, x18-x28}, -40
-# CHECK-ERROR: :[[@LINE+1]]:24: error: invalid register
-cm.pop {ra, x8-x9, x18-x17}, -40
+# CHECK-ERROR: :[[@LINE+1]]:24: error: register must be in the range 'x19' to 'x27'
+cm.pop {x1, x8-x9, x18-x17}, -40
-# CHECK-ERROR: :[[@LINE+1]]:16: error: invalid register
-cm.pop {ra, x8-f8, x18-x17}, -40
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be 'x9'
+cm.pop {x1, x8-f8, x18-x17}, -40
# CHECK-ERROR: :[[@LINE+1]]:15: error: stack adjustment is invalid for this instruction and register list
cm.pop {ra}, -x1
@@ -55,5 +55,47 @@ cm.pop {ra}, -x1
# CHECK-ERROR: :[[@LINE+1]]:15: error: stack adjustment is invalid for this instruction and register list
cm.push {ra}, x1
-# CHECK-ERROR: :[[@LINE+1]]:12: error: register list must end with '}'
+# CHECK-ERROR: :[[@LINE+1]]:12: error: register 'x1' cannot start a multiple register range
cm.push {x1-x9}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:12: error: register 'ra' cannot start a multiple register range
+cm.push {ra-s0}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:13: error: register must be 'x8'
+cm.push {x1,s0}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:13: error: register must be 's0'
+cm.push {ra,x8}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be 'x9'
+cm.push {x1,x8-s1}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be in the range 's1' to 's11'
+cm.push {ra,s0-x9}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: register must be 'x9'
+cm.push {x1,x8-x18}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: register must be 'x18'
+cm.push {x1,x8-x9,s2}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: too many register ranges
+cm.push {ra,s0-s1,x18}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: too many register ranges
+cm.push {ra,s0-s1,s2}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:23: error: register must be in the range 'x19' to 'x27'
+cm.push {x1,x8-x9,x18-s3}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:27: error: too many register ranges
+cm.push {x1,x8-x9,x18-x19,x20}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:19: error: too many register ranges
+cm.push {ra,s0-s1,s3}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:18: error: expected ',' or '}'
+cm.push {ra,s0-s1-s2}, -32
+
+# CHECK-ERROR: :[[@LINE+1]]:16: error: expected ',' or '}'
+cm.push {ra, s0+s11}, -32
``````````
</details>
https://github.com/llvm/llvm-project/pull/134938
More information about the llvm-commits
mailing list