[RISCV] Don't convert virtual register Register to MCRegister in isCo…

…mpressibleInst. (llvm#122843) Calling MCRegisterClass::contains with a Register does an implicit conversion from Register to MCRegister. I think MCRegister is only intended to be used for physical registers. We should protect this implicit conversion by checking for physical registers first. While I was here I removed some unnecessary parentheses from the output.
paulhuggett · Jan 14, 2025 · 726cfc6 · 726cfc6
1 parent 87d7aeb
commit 726cfc6
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 17 deletions.
diff --git a/llvm/test/TableGen/AsmPredicateCombiningRISCV.td b/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
@@ -60,40 +60,71 @@ def BigInst : RVInst<1, [AsmPred1]>;
 def SmallInst1 : RVInst16<1, []>;
 def : CompressPat<(BigInst Regs:$r), (SmallInst1 Regs:$r), [AsmPred1]>;
 // COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond1] &&
-// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
-// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
+// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
 // COMPRESS-NEXT: // SmallInst1 $r
 
 def SmallInst2 : RVInst16<2, []>;
 def : CompressPat<(BigInst Regs:$r), (SmallInst2 Regs:$r), [AsmPred2]>;
 // COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond2a] &&
 // COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
-// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
-// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
+// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
 // COMPRESS-NEXT: // SmallInst2 $r
 
 def SmallInst3 : RVInst16<2, []>;
 def : CompressPat<(BigInst Regs:$r), (SmallInst3 Regs:$r), [AsmPred3]>;
 // COMPRESS:      if ((STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
-// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
-// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
+// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
 // COMPRESS-NEXT: // SmallInst3 $r
 
 def SmallInst4 : RVInst16<2, []>;
 def : CompressPat<(BigInst Regs:$r), (SmallInst4 Regs:$r), [AsmPred1, AsmPred2]>;
 // COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond1] &&
 // COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2a] &&
 // COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
-// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
-// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
+// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
 // COMPRESS-NEXT: // SmallInst4 $r
 
 def SmallInst5 : RVInst16<2, []>;
 def : CompressPat<(BigInst Regs:$r), (SmallInst5 Regs:$r), [AsmPred1, AsmPred3]>;
 // COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond1] &&
 // COMPRESS-NEXT: (STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
-// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
-// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
+// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
 // COMPRESS-NEXT: // SmallInst5 $r
 
 // COMPRESS-LABEL: static bool uncompressInst
+
+// COMPRESS-LABEL: static bool isCompressibleInst
+
+// COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond1] &&
+// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
+// COMPRESS-NEXT: // SmallInst1 $r
+
+// COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond2a] &&
+// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
+// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
+// COMPRESS-NEXT: // SmallInst2 $r
+
+// COMPRESS:      if ((STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
+// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
+// COMPRESS-NEXT: // SmallInst3 $r
+
+// COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond1] &&
+// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2a] &&
+// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
+// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
+// COMPRESS-NEXT: // SmallInst4 $r
+
+// COMPRESS:      if (STI.getFeatureBits()[arch::AsmCond1] &&
+// COMPRESS-NEXT: (STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
+// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
+// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
+// COMPRESS-NEXT: // SmallInst5 $r
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -773,13 +773,17 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
           // This is a register operand. Check the register class.
           // Don't check register class if this is a tied operand, it was done
           // for the operand its tied to.
-          if (DestOperand.getTiedRegister() == -1)
-            CondStream.indent(6)
-                << "(MI.getOperand(" << OpIdx << ").isReg()) &&\n"
-                << "      (" << TargetName << "MCRegisterClasses[" << TargetName
-                << "::" << ClassRec->getName()
-                << "RegClassID].contains(MI.getOperand(" << OpIdx
-                << ").getReg())) &&\n";
+          if (DestOperand.getTiedRegister() == -1) {
+            CondStream.indent(6) << "MI.getOperand(" << OpIdx << ").isReg()";
+            if (EType == EmitterType::CheckCompress)
+              CondStream << " && MI.getOperand(" << OpIdx
+                         << ").getReg().isPhysical()";
+            CondStream << " &&\n"
+                       << indent(6) << TargetName << "MCRegisterClasses["
+                       << TargetName << "::" << ClassRec->getName()
+                       << "RegClassID].contains(MI.getOperand(" << OpIdx
+                       << ").getReg()) &&\n";
+          }
 
           if (CompressOrUncompress)
             CodeStream.indent(6)