[llvm] [RISCV] Don't convert virtual register Register to MCRegister in isCompressibleInst. (PR #122843)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 22:32:08 PST 2025
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/122843
>From 511d0c1451741c35dd9742481a7447a8845d7ed2 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 13 Jan 2025 17:30:24 -0800
Subject: [PATCH 1/3] [RISCV] Don't convert virtual register Register to
MCRegister in isCompressibleInst.
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.
---
.../TableGen/AsmPredicateCombiningRISCV.td | 20 +++++++++----------
llvm/utils/TableGen/CompressInstEmitter.cpp | 13 ++++++++----
2 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/llvm/test/TableGen/AsmPredicateCombiningRISCV.td b/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
index 57ed00583db14c..56622e0f34e022 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,16 @@ 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
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 7ebfe50a86d0fb..9e78abd7aad83a 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -773,13 +773,18 @@ 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)
+ if (DestOperand.getTiedRegister() == -1) {
CondStream.indent(6)
- << "(MI.getOperand(" << OpIdx << ").isReg()) &&\n"
- << " (" << TargetName << "MCRegisterClasses[" << TargetName
+ << "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";
+ << ").getReg()) &&\n";
+ }
if (CompressOrUncompress)
CodeStream.indent(6)
>From 0bbdfa9030bca5fd76591bd9dfbb37ff44541d6c Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 13 Jan 2025 19:35:24 -0800
Subject: [PATCH 2/3] fixup! clang-format
---
llvm/utils/TableGen/CompressInstEmitter.cpp | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/llvm/utils/TableGen/CompressInstEmitter.cpp b/llvm/utils/TableGen/CompressInstEmitter.cpp
index 9e78abd7aad83a..1def8b1ab2290e 100644
--- a/llvm/utils/TableGen/CompressInstEmitter.cpp
+++ b/llvm/utils/TableGen/CompressInstEmitter.cpp
@@ -774,16 +774,15 @@ void CompressInstEmitter::emitCompressInstEmitter(raw_ostream &OS,
// 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()";
+ 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";
+ CondStream << " && MI.getOperand(" << OpIdx
+ << ").getReg().isPhysical()";
+ CondStream << " &&\n"
+ << indent(6) << TargetName << "MCRegisterClasses["
+ << TargetName << "::" << ClassRec->getName()
+ << "RegClassID].contains(MI.getOperand(" << OpIdx
+ << ").getReg()) &&\n";
}
if (CompressOrUncompress)
>From 656527230726c21f31750396d9a55e9b264b86ba Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 13 Jan 2025 22:31:45 -0800
Subject: [PATCH 3/3] fixup! check the isCompressibleInst lines in
AsmPredicateCombiningRISCV.td
---
.../TableGen/AsmPredicateCombiningRISCV.td | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/llvm/test/TableGen/AsmPredicateCombiningRISCV.td b/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
index 56622e0f34e022..d93d3e767735f3 100644
--- a/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
+++ b/llvm/test/TableGen/AsmPredicateCombiningRISCV.td
@@ -97,3 +97,34 @@ def : CompressPat<(BigInst Regs:$r), (SmallInst5 Regs:$r), [AsmPred1, AsmPred3]>
// 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
More information about the llvm-commits
mailing list