[RISCV] Move checking for constant 3/4 for XTHeadMemPair to the instruction matching stage. #136165

topperc · 2025-04-17T17:08:20Z

This removes a special case from processInstruction and removes an untested range diagnostic we would print if the constant didn't fit in 3 bits.

…ching stage. This removes a special case from processInstruction and removes an untested range diagnostic we would print if the constant didn't fit in 3 bits.

llvmbot · 2025-04-17T17:09:02Z

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

@llvm/pr-subscribers-mc

Author: Craig Topper (topperc)

Changes

This removes a special case from processInstruction and removes an untested range diagnostic we would print if the constant didn't fit in 3 bits.

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

5 Files Affected:

(modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+8-13)
(modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h (+2)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+6)
(modified) llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td (+33-9)
(modified) llvm/test/MC/RISCV/rv32xtheadmempair-invalid.s (+2-2)

diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index f8df7ee4c5019..d0a561656fa56 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -932,6 +932,14 @@ struct RISCVOperand final : public MCParsedAsmOperand {
     return isUImmPred([](int64_t Imm) { return 0 == Imm; });
   }
 
+  bool isImmThree() const {
+    return isUImmPred([](int64_t Imm) { return 3 == Imm; });
+  }
+
+  bool isImmFour() const {
+    return isUImmPred([](int64_t Imm) { return 4 == Imm; });
+  }
+
   bool isSImm5Plus1() const {
     return isSImmPred(
         [](int64_t Imm) { return Imm != INT64_MIN && isInt<5>(Imm - 1); });
@@ -3660,19 +3668,6 @@ bool RISCVAsmParser::validateInstruction(MCInst &Inst,
     }
   }
 
-  bool IsTHeadMemPair32 = (Opcode == RISCV::TH_LWD ||
-                           Opcode == RISCV::TH_LWUD || Opcode == RISCV::TH_SWD);
-  bool IsTHeadMemPair64 = (Opcode == RISCV::TH_LDD || Opcode == RISCV::TH_SDD);
-  // The last operand of XTHeadMemPair instructions must be constant 3 or 4
-  // depending on the data width.
-  if (IsTHeadMemPair32 && Inst.getOperand(4).getImm() != 3) {
-    SMLoc Loc = Operands.back()->getStartLoc();
-    return Error(Loc, "operand must be constant 3");
-  } else if (IsTHeadMemPair64 && Inst.getOperand(4).getImm() != 4) {
-    SMLoc Loc = Operands.back()->getStartLoc();
-    return Error(Loc, "operand must be constant 4");
-  }
-
   const MCInstrDesc &MCID = MII.get(Opcode);
   if (!(MCID.TSFlags & RISCVII::ConstraintMask))
     return false;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 84a23de6b1995..6ef94fb5e93da 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -329,6 +329,8 @@ enum OperandType : unsigned {
   OPERAND_UIMM48,
   OPERAND_UIMM64,
   OPERAND_ZERO,
+  OPERAND_THREE,
+  OPERAND_FOUR,
   OPERAND_SIMM5,
   OPERAND_SIMM5_NONZERO,
   OPERAND_SIMM5_PLUS1,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 5d661a3438b1c..04ad56ea83230 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2645,6 +2645,12 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
         case RISCVOp::OPERAND_ZERO:
           Ok = Imm == 0;
           break;
+        case RISCVOp::OPERAND_THREE:
+          Ok = Imm == 3;
+          break;
+        case RISCVOp::OPERAND_FOUR:
+          Ok = Imm == 4;
+          break;
           // clang-format off
         CASE_OPERAND_SIMM(5)
         CASE_OPERAND_SIMM(6)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td
index f69b24ae7d9c5..b1cd2202b9be8 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXTHead.td
@@ -34,6 +34,30 @@ def th_swd : SDNode<"RISCVISD::TH_SWD", SDT_StorePair,
 def th_sdd : SDNode<"RISCVISD::TH_SDD", SDT_StorePair,
                     [SDNPHasChain, SDNPMayStore, SDNPMemOperand]>;
 
+def ImmThreeAsmOperand : AsmOperandClass {
+  let Name = "ImmThree";
+  let RenderMethod = "addImmOperands";
+  let DiagnosticType = !strconcat("Invalid", Name);
+  let DiagnosticString = "operand must be constant 3";
+}
+
+def immthree : RISCVOp {
+  let ParserMatchClass = ImmThreeAsmOperand;
+  let OperandType = "OPERAND_THREE";
+}
+
+def ImmFourAsmOperand : AsmOperandClass {
+  let Name = "ImmFour";
+  let RenderMethod = "addImmOperands";
+  let DiagnosticType = !strconcat("Invalid", Name);
+  let DiagnosticString = "operand must be constant 4";
+}
+
+def immfour : RISCVOp {
+  let ParserMatchClass = ImmFourAsmOperand;
+  let OperandType = "OPERAND_FOUR";
+}
+
 //===----------------------------------------------------------------------===//
 // Instruction class templates
 //===----------------------------------------------------------------------===//
@@ -131,10 +155,10 @@ class THMulAccumulate_rr<bits<7> funct7, string opcodestr>
 }
 
 let hasSideEffects = 0, mayLoad = 1, mayStore = 0 in
-class THLoadPair<bits<5> funct5, string opcodestr>
+class THLoadPair<bits<5> funct5, string opcodestr, Operand consttype>
   : RVInstRBase<0b100, OPC_CUSTOM_0,
                 (outs GPR:$rd, GPR:$rs2),
-                (ins GPR:$rs1, uimm2:$uimm2, uimm7:$const3or4),
+                (ins GPR:$rs1, uimm2:$uimm2, consttype:$const3or4),
                  opcodestr, "$rd, $rs2, (${rs1}), $uimm2, $const3or4"> {
   bits<2> uimm2;
   let Inst{31-27} = funct5;
@@ -144,9 +168,9 @@ class THLoadPair<bits<5> funct5, string opcodestr>
 }
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 1 in
-class THStorePair<bits<5> funct5, string opcodestr>
+class THStorePair<bits<5> funct5, string opcodestr, Operand consttype>
   : RVInstRBase<0b101, OPC_CUSTOM_0, (outs),
-              (ins GPR:$rd, GPR:$rs2, GPR:$rs1, uimm2:$uimm2, uimm7:$const3or4),
+              (ins GPR:$rd, GPR:$rs2, GPR:$rs1, uimm2:$uimm2, consttype:$const3or4),
               opcodestr, "$rd, $rs2, (${rs1}), $uimm2, $const3or4"> {
   bits<2> uimm2;
   let Inst{31-27} = funct5;
@@ -290,19 +314,19 @@ def TH_MULSW : THMulAccumulate_rr<0b0010011, "th.mulsw">;
 } // Predicates = [HasVendorXTHeadMac, IsRV64]
 
 let Predicates = [HasVendorXTHeadMemPair] in {
-def TH_LWUD : THLoadPair<0b11110, "th.lwud">,
+def TH_LWUD : THLoadPair<0b11110, "th.lwud", immthree>,
               Sched<[WriteLDW, WriteLDW, ReadMemBase]>;
-def TH_SWD  : THStorePair<0b11100, "th.swd">,
+def TH_SWD  : THStorePair<0b11100, "th.swd", immthree>,
               Sched<[WriteSTW, WriteSTW, ReadStoreData, ReadMemBase]>;
 let IsSignExtendingOpW = 1 in
-def TH_LWD  : THLoadPair<0b11100, "th.lwd">,
+def TH_LWD  : THLoadPair<0b11100, "th.lwd", immthree>,
               Sched<[WriteLDW, WriteLDW, ReadMemBase]>;
 }
 
 let Predicates = [HasVendorXTHeadMemPair, IsRV64] in {
-def TH_LDD : THLoadPair<0b11111, "th.ldd">,
+def TH_LDD : THLoadPair<0b11111, "th.ldd", immfour>,
              Sched<[WriteLDD, WriteLDD, ReadMemBase]>;
-def TH_SDD : THStorePair<0b11111, "th.sdd">,
+def TH_SDD : THStorePair<0b11111, "th.sdd", immfour>,
              Sched<[WriteSTD, WriteSTD, ReadStoreData, ReadMemBase]>;
 }
 
diff --git a/llvm/test/MC/RISCV/rv32xtheadmempair-invalid.s b/llvm/test/MC/RISCV/rv32xtheadmempair-invalid.s
index 9124218c1f8f5..a9223ae128a40 100644
--- a/llvm/test/MC/RISCV/rv32xtheadmempair-invalid.s
+++ b/llvm/test/MC/RISCV/rv32xtheadmempair-invalid.s
@@ -2,10 +2,10 @@
 
 th.ldd t0, t1, (t2), 5, 4   # CHECK: [[@LINE]]:22: error: invalid operand for instruction
 th.ldd t0, t1, (t2)         # CHECK: [[@LINE]]:1: error: too few operands for instruction
-th.ldd t0, t1, (t2), 3, 5   # CHECK: [[@LINE]]:1: error: instruction requires the following: RV64I Base Instruction Set{{$}}
+th.ldd t0, t1, (t2), 3, 5   # CHECK: [[@LINE]]:25: error: invalid operand for instruction
 th.sdd a0, a1, (a2), 5, 4   # CHECK: [[@LINE]]:22: error: invalid operand for instruction
 th.sdd a0, a1, (a2)         # CHECK: [[@LINE]]:1: error: too few operands for instruction
-th.sdd a0, a1, (a2), 3, 5   # CHECK: [[@LINE]]:1: error: instruction requires the following: RV64I Base Instruction Set{{$}}
+th.sdd a0, a1, (a2), 3, 5   # CHECK: [[@LINE]]:25: error: invalid operand for instruction
 th.lwud t0, t1, (t2), 5, 4  # CHECK: [[@LINE]]:23: error: immediate must be an integer in the range [0, 3]
 th.lwud t0, t1, (t2)        # CHECK: [[@LINE]]:1: error: too few operands for instruction
 th.lwud t0, t1, (t2), 3, 5  # CHECK: [[@LINE]]:26: error: operand must be constant 3

wangpc-pp

LGTM.

llvm-ci · 2025-04-18T04:33:33Z

LLVM Buildbot has detected a new failure on builder ml-opt-devrel-x86-64 running on ml-opt-devrel-x86-64-b2 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/17108