[RISCV] Don't allow '-' after 'ra' in Zcmp/Xqccmp register list.

[topperc]

Move the parsing of minus under the check that we parsed a comma. Unfortunately, this leads to a poor error, but I still have more known issues in this code and may end up with an overall restructuring and want to think about wording.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-mc

Author: Craig Topper (topperc)

Changes

Move the parsing of minus under the check that we parsed a comma. Unfortunately, this leads to a poor error, but I still have more known issues in this code and may end up with an overall restructuring and want to think about wording.

---

Full diff: https://github.com/llvm/llvm-project/pull/134182.diff

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+33-33)
• (modified) llvm/test/MC/RISCV/rv64zcmp-invalid.s (+3)

diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 7837504751694..fe4dc2c996756 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -2612,46 +2612,46 @@ ParseStatus RISCVAsmParser::parseRegListCommon(OperandVector &Operands,
       return Error(getLoc(),
                    "continuous register list must start from 's0' or 'x8'");
     getLexer().Lex(); // eat reg
-  }
-
-  // parse case like -s1
-  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();
-  }
 
-  // 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'");
+    // parse case like -s1
+    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();
+    }
 
-    // parse ', x18' for extra part
-    if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
-      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 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'");
 
-    // parse '-x20' for extra part
-    if (parseOptionalToken(AsmToken::Minus)) {
+      // parse ', x18' for extra part
       if (getLexer().isNot(AsmToken::Identifier) || IsRVE)
         return Error(getLoc(), "invalid register");
-      EndName = getLexer().getTok().getIdentifier();
+      StringRef EndName = getLexer().getTok().getIdentifier();
       RegEnd = MatchRegisterName(EndName);
-      if (!(RegEnd >= RISCV::X19 && RegEnd <= RISCV::X27))
-        return Error(getLoc(), "invalid register");
+      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();
+      }
     }
   }
 
diff --git a/llvm/test/MC/RISCV/rv64zcmp-invalid.s b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
index ffaffdf6a5999..c66415cb49b34 100644
--- a/llvm/test/MC/RISCV/rv64zcmp-invalid.s
+++ b/llvm/test/MC/RISCV/rv64zcmp-invalid.s
@@ -54,3 +54,6 @@ 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 '}'
+cm.push {x1-x9}, -32

[lenary]

LGTM

I wonder if the easiest way to restructure is to have a function parseSingleRegisterOrRange which we can invoke (up to) twice, depending on what range it got to?