Skip to content

Commit 726cfc6

Browse files
topperctopperc
authored
[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.
1 parent 87d7aeb commit 726cfc6

File tree

2 files changed

+52
-17
lines changed

2 files changed

+52
-17
lines changed

‎llvm/test/TableGen/AsmPredicateCombiningRISCV.td

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,40 +60,71 @@ def BigInst : RVInst<1, [AsmPred1]>;
6060
def SmallInst1 : RVInst16<1, []>;
6161
def : CompressPat<(BigInst Regs:$r), (SmallInst1 Regs:$r), [AsmPred1]>;
6262
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond1] &&
63-
// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
64-
// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
63+
// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
64+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
6565
// COMPRESS-NEXT: // SmallInst1 $r
6666

6767
def SmallInst2 : RVInst16<2, []>;
6868
def : CompressPat<(BigInst Regs:$r), (SmallInst2 Regs:$r), [AsmPred2]>;
6969
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond2a] &&
7070
// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
71-
// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
72-
// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
71+
// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
72+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
7373
// COMPRESS-NEXT: // SmallInst2 $r
7474

7575
def SmallInst3 : RVInst16<2, []>;
7676
def : CompressPat<(BigInst Regs:$r), (SmallInst3 Regs:$r), [AsmPred3]>;
7777
// COMPRESS: if ((STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
78-
// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
79-
// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
78+
// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
79+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
8080
// COMPRESS-NEXT: // SmallInst3 $r
8181

8282
def SmallInst4 : RVInst16<2, []>;
8383
def : CompressPat<(BigInst Regs:$r), (SmallInst4 Regs:$r), [AsmPred1, AsmPred2]>;
8484
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond1] &&
8585
// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2a] &&
8686
// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
87-
// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
88-
// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
87+
// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
88+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
8989
// COMPRESS-NEXT: // SmallInst4 $r
9090

9191
def SmallInst5 : RVInst16<2, []>;
9292
def : CompressPat<(BigInst Regs:$r), (SmallInst5 Regs:$r), [AsmPred1, AsmPred3]>;
9393
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond1] &&
9494
// COMPRESS-NEXT: (STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
95-
// COMPRESS-NEXT: (MI.getOperand(0).isReg()) &&
96-
// COMPRESS-NEXT: (archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg()))) {
95+
// COMPRESS-NEXT: MI.getOperand(0).isReg() &&
96+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
9797
// COMPRESS-NEXT: // SmallInst5 $r
9898

9999
// COMPRESS-LABEL: static bool uncompressInst
100+
101+
// COMPRESS-LABEL: static bool isCompressibleInst
102+
103+
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond1] &&
104+
// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
105+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
106+
// COMPRESS-NEXT: // SmallInst1 $r
107+
108+
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond2a] &&
109+
// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
110+
// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
111+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
112+
// COMPRESS-NEXT: // SmallInst2 $r
113+
114+
// COMPRESS: if ((STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
115+
// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
116+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
117+
// COMPRESS-NEXT: // SmallInst3 $r
118+
119+
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond1] &&
120+
// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2a] &&
121+
// COMPRESS-NEXT: STI.getFeatureBits()[arch::AsmCond2b] &&
122+
// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
123+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
124+
// COMPRESS-NEXT: // SmallInst4 $r
125+
126+
// COMPRESS: if (STI.getFeatureBits()[arch::AsmCond1] &&
127+
// COMPRESS-NEXT: (STI.getFeatureBits()[arch::AsmCond3a] || STI.getFeatureBits()[arch::AsmCond3b]) &&
128+
// COMPRESS-NEXT: MI.getOperand(0).isReg() && MI.getOperand(0).getReg().isPhysical() &&
129+
// COMPRESS-NEXT: archMCRegisterClasses[arch::RegsRegClassID].contains(MI.getOperand(0).getReg())) {
130+
// COMPRESS-NEXT: // SmallInst5 $r

‎llvm/utils/TableGen/CompressInstEmitter.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -773,13 +773,17 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
773773
// This is a register operand. Check the register class.
774774
// Don't check register class if this is a tied operand, it was done
775775
// for the operand its tied to.
776-
if (DestOperand.getTiedRegister() == -1)
777-
CondStream.indent(6)
778-
<< "(MI.getOperand(" << OpIdx << ").isReg()) &&\n"
779-
<< " (" << TargetName << "MCRegisterClasses[" << TargetName
780-
<< "::" << ClassRec->getName()
781-
<< "RegClassID].contains(MI.getOperand(" << OpIdx
782-
<< ").getReg())) &&\n";
776+
if (DestOperand.getTiedRegister() == -1) {
777+
CondStream.indent(6) << "MI.getOperand(" << OpIdx << ").isReg()";
778+
if (EType == EmitterType::CheckCompress)
779+
CondStream << " && MI.getOperand(" << OpIdx
780+
<< ").getReg().isPhysical()";
781+
CondStream << " &&\n"
782+
<< indent(6) << TargetName << "MCRegisterClasses["
783+
<< TargetName << "::" << ClassRec->getName()
784+
<< "RegClassID].contains(MI.getOperand(" << OpIdx
785+
<< ").getReg()) &&\n";
786+
}
783787

784788
if (CompressOrUncompress)
785789
CodeStream.indent(6)

0 commit comments

Comments
 (0)