[RISCV][MC] Support imm symbol in parseCSRSystemRegister

[zqb-all]

No description provided.

[llvmbot]

@llvm/pr-subscribers-mc

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

Author: Mark Zhuang (zqb-all)

Changes

---

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

3 Files Affected:

• (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+43-41)
• (modified) llvm/test/MC/RISCV/rv32i-invalid.s (+7-7)
• (modified) llvm/test/MC/RISCV/rv32i-valid.s (+3)

diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index e68674e830436f..95f3fc0d473238 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -1877,10 +1877,13 @@ ParseStatus RISCVAsmParser::parseInsnCDirectiveOpcode(OperandVector &Operands) {
 ParseStatus RISCVAsmParser::parseCSRSystemRegister(OperandVector &Operands) {
   SMLoc S = getLoc();
   const MCExpr *Res;
+  auto Kind = getLexer().getKind();
 
-  switch (getLexer().getKind()) {
+  switch (Kind) {
   default:
     return ParseStatus::NoMatch;
+
+  case AsmToken::Identifier:
   case AsmToken::LParen:
   case AsmToken::Minus:
   case AsmToken::Plus:
@@ -1891,6 +1894,45 @@ ParseStatus RISCVAsmParser::parseCSRSystemRegister(OperandVector &Operands) {
     if (getParser().parseExpression(Res))
       return ParseStatus::Failure;
 
+    if (Kind == AsmToken::Identifier) {
+      auto SRE = dyn_cast<MCSymbolRefExpr>(Res);
+      if (SRE) {
+        auto Identifier = SRE->getSymbol().getName();
+        const auto *SysReg = RISCVSysReg::lookupSysRegByName(Identifier);
+        if (!SysReg)
+          SysReg = RISCVSysReg::lookupSysRegByAltName(Identifier);
+        if (!SysReg)
+          if ((SysReg = RISCVSysReg::lookupSysRegByDeprecatedName(Identifier)))
+            Warning(S, "'" + Identifier + "' is a deprecated alias for '" +
+                           SysReg->Name + "'");
+
+        // Accept a named Sys Reg if the required features are present.
+        if (SysReg) {
+          const auto &FeatureBits = getSTI().getFeatureBits();
+          if (!SysReg->haveRequiredFeatures(FeatureBits)) {
+            const auto *Feature = llvm::find_if(RISCVFeatureKV, [&](auto Feature) {
+              return SysReg->FeaturesRequired[Feature.Value];
+            });
+            auto ErrorMsg = std::string("system register '") + SysReg->Name + "' ";
+            if (SysReg->isRV32Only && FeatureBits[RISCV::Feature64Bit]) {
+              ErrorMsg += "is RV32 only";
+              if (Feature != std::end(RISCVFeatureKV))
+                ErrorMsg += " and ";
+            }
+            if (Feature != std::end(RISCVFeatureKV)) {
+              ErrorMsg +=
+                  "requires '" + std::string(Feature->Key) + "' to be enabled";
+            }
+
+            return Error(S, ErrorMsg);
+          }
+          Operands.push_back(
+              RISCVOperand::createSysReg(Identifier, S, SysReg->Encoding));
+          return ParseStatus::Success;
+        }
+      }
+    }
+
     auto *CE = dyn_cast<MCConstantExpr>(Res);
     if (CE) {
       int64_t Imm = CE->getValue();
@@ -1911,46 +1953,6 @@ ParseStatus RISCVAsmParser::parseCSRSystemRegister(OperandVector &Operands) {
       }
     }
 
-    return generateImmOutOfRangeError(S, 0, (1 << 12) - 1);
-  }
-  case AsmToken::Identifier: {
-    StringRef Identifier;
-    if (getParser().parseIdentifier(Identifier))
-      return ParseStatus::Failure;
-
-    const auto *SysReg = RISCVSysReg::lookupSysRegByName(Identifier);
-    if (!SysReg)
-      SysReg = RISCVSysReg::lookupSysRegByAltName(Identifier);
-    if (!SysReg)
-      if ((SysReg = RISCVSysReg::lookupSysRegByDeprecatedName(Identifier)))
-        Warning(S, "'" + Identifier + "' is a deprecated alias for '" +
-                       SysReg->Name + "'");
-
-    // Accept a named Sys Reg if the required features are present.
-    if (SysReg) {
-      const auto &FeatureBits = getSTI().getFeatureBits();
-      if (!SysReg->haveRequiredFeatures(FeatureBits)) {
-        const auto *Feature = llvm::find_if(RISCVFeatureKV, [&](auto Feature) {
-          return SysReg->FeaturesRequired[Feature.Value];
-        });
-        auto ErrorMsg = std::string("system register '") + SysReg->Name + "' ";
-        if (SysReg->isRV32Only && FeatureBits[RISCV::Feature64Bit]) {
-          ErrorMsg += "is RV32 only";
-          if (Feature != std::end(RISCVFeatureKV))
-            ErrorMsg += " and ";
-        }
-        if (Feature != std::end(RISCVFeatureKV)) {
-          ErrorMsg +=
-              "requires '" + std::string(Feature->Key) + "' to be enabled";
-        }
-
-        return Error(S, ErrorMsg);
-      }
-      Operands.push_back(
-          RISCVOperand::createSysReg(Identifier, S, SysReg->Encoding));
-      return ParseStatus::Success;
-    }
-
     return generateImmOutOfRangeError(S, 0, (1 << 12) - 1,
                                       "operand must be a valid system register "
                                       "name or an integer in the range");
diff --git a/llvm/test/MC/RISCV/rv32i-invalid.s b/llvm/test/MC/RISCV/rv32i-invalid.s
index 80a59df94e36a3..a33b53b8237e5f 100644
--- a/llvm/test/MC/RISCV/rv32i-invalid.s
+++ b/llvm/test/MC/RISCV/rv32i-invalid.s
@@ -21,13 +21,13 @@ ori a0, a1, -2049 # CHECK: :[[@LINE]]:13: error: operand must be a symbol with %
 andi ra, sp, 2048 # CHECK: :[[@LINE]]:14: error: operand must be a symbol with %lo/%pcrel_lo/%tprel_lo modifier or an integer in the range [-2048, 2047]
 
 ## uimm12
-csrrw a0, -1, a0 # CHECK: :[[@LINE]]:11: error: immediate must be an integer in the range [0, 4095]
-csrrs a0, 4096, a0 # CHECK: :[[@LINE]]:11: error: immediate must be an integer in the range [0, 4095]
-csrrs a0, -0xf, a0 # CHECK: :[[@LINE]]:11: error: immediate must be an integer in the range [0, 4095]
-csrrc a0, 0x1000, a0 # CHECK: :[[@LINE]]:11: error: immediate must be an integer in the range [0, 4095]
-csrrwi a0, -50, 0 # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [0, 4095]
-csrrsi a0, 4097, a0 # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [0, 4095]
-csrrci a0, 0xffff, a0 # CHECK: :[[@LINE]]:12: error: immediate must be an integer in the range [0, 4095]
+csrrw a0, -1, a0 # CHECK: :[[@LINE]]:11: error: operand must be a valid system register name or an integer in the range [0, 4095]
+csrrs a0, 4096, a0 # CHECK: :[[@LINE]]:11: error: operand must be a valid system register name or an integer in the range [0, 4095]
+csrrs a0, -0xf, a0 # CHECK: :[[@LINE]]:11: error: operand must be a valid system register name or an integer in the range [0, 4095]
+csrrc a0, 0x1000, a0 # CHECK: :[[@LINE]]:11: error: operand must be a valid system register name or an integer in the range [0, 4095]
+csrrwi a0, -50, 0 # CHECK: :[[@LINE]]:12: error: operand must be a valid system register name or an integer in the range [0, 4095]
+csrrsi a0, 4097, a0 # CHECK: :[[@LINE]]:12: error: operand must be a valid system register name or an integer in the range [0, 4095]
+csrrci a0, 0xffff, a0 # CHECK: :[[@LINE]]:12: error: operand must be a valid system register name or an integer in the range [0, 4095]
 
 ## simm13_lsb0
 beq t0, t1, -4098 # CHECK: :[[@LINE]]:13: error: immediate must be a multiple of 2 bytes in the range [-4096, 4094]
diff --git a/llvm/test/MC/RISCV/rv32i-valid.s b/llvm/test/MC/RISCV/rv32i-valid.s
index f03c2e1c23cf3b..c5bad80e79de2d 100644
--- a/llvm/test/MC/RISCV/rv32i-valid.s
+++ b/llvm/test/MC/RISCV/rv32i-valid.s
@@ -373,3 +373,6 @@ csrrsi t2, 0xfff, 31
 # CHECK-ASM-AND-OBJ: csrrci t1, sscratch, 5
 # CHECK-ASM: encoding: [0x73,0xf3,0x02,0x14]
 csrrci t1, 0x140, 5
+# CHECK-ASM-AND-OBJ: csrrw t0, 16, t1
+# CHECK-ASM: encoding: [0xf3,0x12,0x03,0x01]
+csrrw t0, CONST, t1

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[arichardson]

I did something similar in #67377 but forgot about this change so didn't update it. It might be a good idea to see if you can reuse any of the tests there.

[zqb-all]

I did something similar in #67377 but forgot about this change so didn't update it. It might be a good idea to see if you can reuse any of the tests there.

Thanks! I found that my implementation would not prefer use the CSR name when the .set and CSR name conflicted.
So eventually I used your implementation in the #67377.

[arichardson]

This looks good to me. Since this uses some of my code could you please add Co-authored-by: Alex Richardson <[email protected]> to the commit message.

[zqb-all]

This looks good to me. Since this uses some of my code could you please add Co-authored-by: Alex Richardson <[email protected]> to the commit message.

Oh, of course. Sorry I didn't add it before, it's now added, thanks again for the code and review.

[zqb-all]

ping

[wangpc-pp]

LGTM.