[llvm] 726cfc6 - [RISCV] Don't convert virtual register Register to MCRegister in isCompressibleInst. (#122843)

[via llvm-commits]

Author: Craig Topper
Date: 2025-01-13T23:36:09-08:00
New Revision: 726cfc67b69633119279a6369263491421861b1d

URL: https://github.com/llvm/llvm-project/commit/726cfc67b69633119279a6369263491421861b1d
DIFF: https://github.com/llvm/llvm-project/commit/726cfc67b69633119279a6369263491421861b1d.diff

LOG: [RISCV] Don't convert virtual register Register to MCRegister in isCompressibleInst. (#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.

Added: 
    

Modified: 
    llvm/test/TableGen/AsmPredicateCombiningRISCV.td
    llvm/utils/TableGen/CompressInstEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/TableGen/AsmPredicateCombiningRISCV.td b/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
index 57ed00583db14c..d93d3e767735f3 100644
--- a/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
+++ b/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
@@ -60,23 +60,23 @@ 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, []>;
@@ -84,16 +84,47 @@ 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
index 7ebfe50a86d0fb..1def8b1ab2290e 100644
--- 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)