[llvm] [TableGen] Add isReg check for CheckRegOperand predicate (PR #215230)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 10 02:51:55 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-risc-v
Author: renndong
<details>
<summary>Changes</summary>
CheckRegOperand and CheckRegOperandSimple currently call getReg() without first verifying that the operand is a register. Users must use CheckIsRegOperand explicitly to avoid errors for other operand kinds, as exposed by #<!-- -->213815.
This PR adds isReg() check for two predicates before accessing the register value. And remove redundant CheckIsRegOperand checks from existing users for x86, ARM and RISCV backend.
---
Full diff: https://github.com/llvm/llvm-project/pull/215230.diff
7 Files Affected:
- (modified) llvm/include/llvm/Target/TargetInstrPredicate.td (+9-4)
- (modified) llvm/lib/Target/AArch64/AArch64SchedPredicates.td (+37-48)
- (modified) llvm/lib/Target/ARM/ARMScheduleM85.td (+1-3)
- (modified) llvm/lib/Target/RISCV/RISCVInstrPredicates.td (-4)
- (modified) llvm/lib/Target/RISCV/RISCVMacroFusionXQCI.td (-5)
- (modified) llvm/test/TableGen/MacroFusion.td (+5-5)
- (modified) llvm/utils/TableGen/Common/PredicateExpander.cpp (+11-4)
``````````diff
diff --git a/llvm/include/llvm/Target/TargetInstrPredicate.td b/llvm/include/llvm/Target/TargetInstrPredicate.td
index b5419cb9f3867..21ef96b95bfb8 100644
--- a/llvm/include/llvm/Target/TargetInstrPredicate.td
+++ b/llvm/include/llvm/Target/TargetInstrPredicate.td
@@ -28,7 +28,12 @@
//
// Every MCInstPredicate class has a well-known semantic in tablegen. For
// example, `CheckOpcode` is a special type of predicate used to describe a
-// constraint on the value of an instruction opcode.
+// constraint on the value of an instruction opcode, while `CheckIsRegOperand`
+// checks whether an instruction operand is a register operand.
+// `CheckRegOperand` performs this check implicitly before comparing the
+// register value. Therefore, a preceding `CheckIsRegOperand` is unnecessary
+// when we don't care about the type of the operand (e.g. an immediate or a
+// register) but only care the fact that it is a target register.
//
// MCInstPredicate definitions are typically used by scheduling models to
// construct MCSchedPredicate definitions (see the definition of class
@@ -44,7 +49,7 @@
// def M3BranchLinkFastPred : SchedPredicate<[{
// MI->getOpcode() == AArch64::BLR &&
// MI->getOperand(0).isReg() &&
-// MI->getOperand(0).getReg() != AArch64::LR}]>;
+// !(MI->getOperand(0).isReg() && MI->getOperand(0).getReg() == AArch64::LR)}]>;
//
// The main advantage of using MCInstPredicate instead of SchedPredicate is
// portability: users don't need to specify predicates in C++. As a consequence
@@ -123,8 +128,8 @@ class CheckOperandBase<int Index, string Fn = ""> : MCOperandPredicate<Index> {
}
// Check that the machine register operand at position `Index` references
-// register R. This predicate assumes that we already checked that the machine
-// operand at position `Index` is a register operand.
+// register R. This predicate checks whether the operand at position `Index`
+// is a register first and return false when not satisfied.
class CheckRegOperand<int Index, Register R> : CheckOperandBase<Index> {
Register Reg = R;
}
diff --git a/llvm/lib/Target/AArch64/AArch64SchedPredicates.td b/llvm/lib/Target/AArch64/AArch64SchedPredicates.td
index 870a82bdffd74..cbaaafccd164d 100644
--- a/llvm/lib/Target/AArch64/AArch64SchedPredicates.td
+++ b/llvm/lib/Target/AArch64/AArch64SchedPredicates.td
@@ -294,9 +294,7 @@ def IsCopyIdiomFn : TIIPredicate<"isCopyIdiom",
[ADDWri, ADDXri],
MCReturnStatement<
CheckAll<
- [CheckIsRegOperand<0>,
- CheckIsRegOperand<1>,
- CheckAny<
+ [CheckAny<
[CheckRegOperand<0, WSP>,
CheckRegOperand<0, SP>,
CheckRegOperand<1, WSP>,
@@ -391,40 +389,34 @@ def IsFastSBFMImmPred : MCSchedPredicate<CheckAny<[
IsSBFMASRImm<SBFMXri, 64> ]>>;
// Identify whether destination operand is a W-form register.
-def CheckIsWRegOp0 : CheckAll<[
- CheckIsRegOperand<0>,
- CheckAny<[
- CheckRegOperand<0, W0>, CheckRegOperand<0, W1>, CheckRegOperand<0, W2>,
- CheckRegOperand<0, W3>, CheckRegOperand<0, W4>, CheckRegOperand<0, W5>,
- CheckRegOperand<0, W6>, CheckRegOperand<0, W7>, CheckRegOperand<0, W8>,
- CheckRegOperand<0, W9>, CheckRegOperand<0, W10>, CheckRegOperand<0, W11>,
- CheckRegOperand<0, W12>, CheckRegOperand<0, W13>, CheckRegOperand<0, W14>,
- CheckRegOperand<0, W15>, CheckRegOperand<0, W16>, CheckRegOperand<0, W17>,
- CheckRegOperand<0, W18>, CheckRegOperand<0, W19>, CheckRegOperand<0, W20>,
- CheckRegOperand<0, W21>, CheckRegOperand<0, W22>, CheckRegOperand<0, W23>,
- CheckRegOperand<0, W24>, CheckRegOperand<0, W25>, CheckRegOperand<0, W26>,
- CheckRegOperand<0, W27>, CheckRegOperand<0, W28>, CheckRegOperand<0, W29>,
- CheckRegOperand<0, W30>, CheckRegOperand<0, WZR>, CheckRegOperand<0, WSP>
- ]>
+def CheckIsWRegOp0 : CheckAny<[
+ CheckRegOperand<0, W0>, CheckRegOperand<0, W1>, CheckRegOperand<0, W2>,
+ CheckRegOperand<0, W3>, CheckRegOperand<0, W4>, CheckRegOperand<0, W5>,
+ CheckRegOperand<0, W6>, CheckRegOperand<0, W7>, CheckRegOperand<0, W8>,
+ CheckRegOperand<0, W9>, CheckRegOperand<0, W10>, CheckRegOperand<0, W11>,
+ CheckRegOperand<0, W12>, CheckRegOperand<0, W13>, CheckRegOperand<0, W14>,
+ CheckRegOperand<0, W15>, CheckRegOperand<0, W16>, CheckRegOperand<0, W17>,
+ CheckRegOperand<0, W18>, CheckRegOperand<0, W19>, CheckRegOperand<0, W20>,
+ CheckRegOperand<0, W21>, CheckRegOperand<0, W22>, CheckRegOperand<0, W23>,
+ CheckRegOperand<0, W24>, CheckRegOperand<0, W25>, CheckRegOperand<0, W26>,
+ CheckRegOperand<0, W27>, CheckRegOperand<0, W28>, CheckRegOperand<0, W29>,
+ CheckRegOperand<0, W30>, CheckRegOperand<0, WZR>, CheckRegOperand<0, WSP>
]>;
def IsWFormPred : MCSchedPredicate<CheckIsWRegOp0>;
// Identify whether destination operand is an X-form register.
-def CheckIsXRegOp0 : CheckAll<[
- CheckIsRegOperand<0>,
- CheckAny<[
- CheckRegOperand<0, X0>, CheckRegOperand<0, X1>, CheckRegOperand<0, X2>,
- CheckRegOperand<0, X3>, CheckRegOperand<0, X4>, CheckRegOperand<0, X5>,
- CheckRegOperand<0, X6>, CheckRegOperand<0, X7>, CheckRegOperand<0, X8>,
- CheckRegOperand<0, X9>, CheckRegOperand<0, X10>, CheckRegOperand<0, X11>,
- CheckRegOperand<0, X12>, CheckRegOperand<0, X13>, CheckRegOperand<0, X14>,
- CheckRegOperand<0, X15>, CheckRegOperand<0, X16>, CheckRegOperand<0, X17>,
- CheckRegOperand<0, X18>, CheckRegOperand<0, X19>, CheckRegOperand<0, X20>,
- CheckRegOperand<0, X21>, CheckRegOperand<0, X22>, CheckRegOperand<0, X23>,
- CheckRegOperand<0, X24>, CheckRegOperand<0, X25>, CheckRegOperand<0, X26>,
- CheckRegOperand<0, X27>, CheckRegOperand<0, X28>, CheckRegOperand<0, FP>,
- CheckRegOperand<0, LR>, CheckRegOperand<0, XZR>, CheckRegOperand<0, SP>
- ]>
+def CheckIsXRegOp0 : CheckAny<[
+ CheckRegOperand<0, X0>, CheckRegOperand<0, X1>, CheckRegOperand<0, X2>,
+ CheckRegOperand<0, X3>, CheckRegOperand<0, X4>, CheckRegOperand<0, X5>,
+ CheckRegOperand<0, X6>, CheckRegOperand<0, X7>, CheckRegOperand<0, X8>,
+ CheckRegOperand<0, X9>, CheckRegOperand<0, X10>, CheckRegOperand<0, X11>,
+ CheckRegOperand<0, X12>, CheckRegOperand<0, X13>, CheckRegOperand<0, X14>,
+ CheckRegOperand<0, X15>, CheckRegOperand<0, X16>, CheckRegOperand<0, X17>,
+ CheckRegOperand<0, X18>, CheckRegOperand<0, X19>, CheckRegOperand<0, X20>,
+ CheckRegOperand<0, X21>, CheckRegOperand<0, X22>, CheckRegOperand<0, X23>,
+ CheckRegOperand<0, X24>, CheckRegOperand<0, X25>, CheckRegOperand<0, X26>,
+ CheckRegOperand<0, X27>, CheckRegOperand<0, X28>, CheckRegOperand<0, FP>,
+ CheckRegOperand<0, LR>, CheckRegOperand<0, XZR>, CheckRegOperand<0, SP>
]>;
def IsXFormPred : MCSchedPredicate<CheckIsXRegOp0>;
@@ -432,21 +424,18 @@ def IsXOrWDest : MCSchedPredicate<
CheckAll<[CheckAny<[CheckIsXRegOp0, CheckIsWRegOp0]>]>
>;
-def CheckIsZRegOp0 : CheckAll<[
- CheckIsRegOperand<0>,
- CheckAny<[
- CheckRegOperand<0, Z0>, CheckRegOperand<0, Z1>, CheckRegOperand<0, Z2>,
- CheckRegOperand<0, Z3>, CheckRegOperand<0, Z4>, CheckRegOperand<0, Z5>,
- CheckRegOperand<0, Z6>, CheckRegOperand<0, Z7>, CheckRegOperand<0, Z8>,
- CheckRegOperand<0, Z9>, CheckRegOperand<0, Z10>, CheckRegOperand<0, Z11>,
- CheckRegOperand<0, Z12>, CheckRegOperand<0, Z13>, CheckRegOperand<0, Z14>,
- CheckRegOperand<0, Z15>, CheckRegOperand<0, Z16>, CheckRegOperand<0, Z17>,
- CheckRegOperand<0, Z18>, CheckRegOperand<0, Z19>, CheckRegOperand<0, Z20>,
- CheckRegOperand<0, Z21>, CheckRegOperand<0, Z22>, CheckRegOperand<0, Z23>,
- CheckRegOperand<0, Z24>, CheckRegOperand<0, Z25>, CheckRegOperand<0, Z26>,
- CheckRegOperand<0, Z27>, CheckRegOperand<0, Z28>, CheckRegOperand<0, Z29>,
- CheckRegOperand<0, Z30>, CheckRegOperand<0, Z31>
- ]>
+def CheckIsZRegOp0 : CheckAny<[
+ CheckRegOperand<0, Z0>, CheckRegOperand<0, Z1>, CheckRegOperand<0, Z2>,
+ CheckRegOperand<0, Z3>, CheckRegOperand<0, Z4>, CheckRegOperand<0, Z5>,
+ CheckRegOperand<0, Z6>, CheckRegOperand<0, Z7>, CheckRegOperand<0, Z8>,
+ CheckRegOperand<0, Z9>, CheckRegOperand<0, Z10>, CheckRegOperand<0, Z11>,
+ CheckRegOperand<0, Z12>, CheckRegOperand<0, Z13>, CheckRegOperand<0, Z14>,
+ CheckRegOperand<0, Z15>, CheckRegOperand<0, Z16>, CheckRegOperand<0, Z17>,
+ CheckRegOperand<0, Z18>, CheckRegOperand<0, Z19>, CheckRegOperand<0, Z20>,
+ CheckRegOperand<0, Z21>, CheckRegOperand<0, Z22>, CheckRegOperand<0, Z23>,
+ CheckRegOperand<0, Z24>, CheckRegOperand<0, Z25>, CheckRegOperand<0, Z26>,
+ CheckRegOperand<0, Z27>, CheckRegOperand<0, Z28>, CheckRegOperand<0, Z29>,
+ CheckRegOperand<0, Z30>, CheckRegOperand<0, Z31>
]>;
diff --git a/llvm/lib/Target/ARM/ARMScheduleM85.td b/llvm/lib/Target/ARM/ARMScheduleM85.td
index beeda468397ec..8b5e19501516b 100644
--- a/llvm/lib/Target/ARM/ARMScheduleM85.td
+++ b/llvm/lib/Target/ARM/ARMScheduleM85.td
@@ -605,9 +605,7 @@ let SingleIssue = 1 in {
def M85VMSRLate : SchedWriteRes<[M85UnitVPort]> { let Latency = 3; }
}
-def M85FPSCRFlagPred : MCSchedPredicate<
- CheckAll<[CheckIsRegOperand<0>,
- CheckRegOperand<0, PC>]>>;
+def M85FPSCRFlagPred : MCSchedPredicate<CheckRegOperand<0, PC>>;
def M85VMRSFPSCR : SchedWriteVariant<[
SchedVar<M85FPSCRFlagPred, [M85VMRSEarly]>,
diff --git a/llvm/lib/Target/RISCV/RISCVInstrPredicates.td b/llvm/lib/Target/RISCV/RISCVInstrPredicates.td
index cff834150cfeb..ff045c32759d3 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrPredicates.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrPredicates.td
@@ -43,7 +43,6 @@ def isZEXT_W
MCReturnStatement<CheckAll<[
CheckOpcode<[ADD_UW]>,
CheckIsRegOperand<1>,
- CheckIsRegOperand<2>,
CheckRegOperand<2, X0>
]>>>;
@@ -184,7 +183,6 @@ def isLoadImmediate
: TIIPredicate<"isLoadImmediate",
MCReturnStatement<CheckAll<[
CheckOpcode<[ADDI]>,
- CheckIsRegOperand<1>,
CheckRegOperand<1, X0>,
CheckIsImmOperand<2>
]>>>;
@@ -193,7 +191,6 @@ def isNonZeroLoadImmediate
: TIIPredicate<"isNonZeroLoadImmediate",
MCReturnStatement<CheckAll<[
CheckOpcode<[ADDI]>,
- CheckIsRegOperand<1>,
CheckRegOperand<1, X0>,
CheckIsImmOperand<2>,
CheckNot<CheckImmOperand<2, 0>>
@@ -203,7 +200,6 @@ def isLPAD
: TIIPredicate<"isLPAD",
MCReturnStatement<CheckAll<[
CheckOpcode<[AUIPC]>,
- CheckIsRegOperand<0>,
CheckRegOperand<0, X0>,
]>>>;
diff --git a/llvm/lib/Target/RISCV/RISCVMacroFusionXQCI.td b/llvm/lib/Target/RISCV/RISCVMacroFusionXQCI.td
index 2fedb06bb3006..42c1e5c8e50b8 100644
--- a/llvm/lib/Target/RISCV/RISCVMacroFusionXQCI.td
+++ b/llvm/lib/Target/RISCV/RISCVMacroFusionXQCI.td
@@ -24,7 +24,6 @@ def TuneMOVIMMALUXqciFusion
CheckAny<[
CheckAll<[
CheckOpcode<[ADDI]>,
- CheckIsRegOperand<1>,
CheckRegOperand<1, X0>
]>,
CheckOpcode<LoadImmOp>,
@@ -45,7 +44,6 @@ def TuneMOVIMMMULXqciFusion
CheckAny<[
CheckAll<[
CheckOpcode<[ADDI]>,
- CheckIsRegOperand<1>,
CheckRegOperand<1, X0>
]>,
CheckOpcode<LoadImmOp>,
@@ -59,7 +57,6 @@ def TuneMOVIMMLoadStoreXqciFusion
CheckAny<[
CheckAll<[
CheckOpcode<[ADDI]>,
- CheckIsRegOperand<1>,
CheckRegOperand<1, X0>
]>,
CheckOpcode<LoadImmOp>,
@@ -73,7 +70,6 @@ def TuneMOVIMMJumpXqciFusion
CheckAny<[
CheckAll<[
CheckOpcode<[ADDI]>,
- CheckIsRegOperand<1>,
CheckRegOperand<1, X0>
]>,
CheckOpcode<LoadImmOp>,
@@ -90,7 +86,6 @@ def TuneMOVIMMLongAccumXciFusion
CheckAny<[
CheckAll<[
CheckOpcode<[ADDI]>,
- CheckIsRegOperand<1>,
CheckRegOperand<1, X0>
]>,
CheckOpcode<LoadImmOp>,
diff --git a/llvm/test/TableGen/MacroFusion.td b/llvm/test/TableGen/MacroFusion.td
index da4adf75ac7eb..909298f0f4bd3 100644
--- a/llvm/test/TableGen/MacroFusion.td
+++ b/llvm/test/TableGen/MacroFusion.td
@@ -117,12 +117,12 @@ def TestPostRAOnlyFusion: SimpleFusion<"test-postra-only", "HasTestPostRAOnlyFus
// CHECK-PREDICATOR-NEXT: {{[[]}}{{[[]}}maybe_unused{{[]]}}{{[]]}} auto &MRI = SecondMI.getMF()->getRegInfo();
// CHECK-PREDICATOR-NEXT: {
// CHECK-PREDICATOR-NEXT: {{[[]}}{{[[]}}maybe_unused{{[]]}}{{[]]}} const MachineInstr *MI = FirstMI;
-// CHECK-PREDICATOR-NEXT: if (MI->getOperand(0).getReg() != Test::X0)
+// CHECK-PREDICATOR-NEXT: if (!(MI->getOperand(0).isReg() && MI->getOperand(0).getReg() == Test::X0))
// CHECK-PREDICATOR-NEXT: return false;
// CHECK-PREDICATOR-NEXT: }
// CHECK-PREDICATOR-NEXT: {
// CHECK-PREDICATOR-NEXT: {{[[]}}{{[[]}}maybe_unused{{[]]}}{{[]]}} const MachineInstr *MI = &SecondMI;
-// CHECK-PREDICATOR-NEXT: if (MI->getOperand(0).getReg() != Test::X0)
+// CHECK-PREDICATOR-NEXT: if (!(MI->getOperand(0).isReg() && MI->getOperand(0).getReg() == Test::X0))
// CHECK-PREDICATOR-NEXT: return false;
// CHECK-PREDICATOR-NEXT: }
// CHECK-PREDICATOR-NEXT: if (SecondMI.getMF()->getProperties().hasNoVRegs())
@@ -145,7 +145,7 @@ def TestPostRAOnlyFusion: SimpleFusion<"test-postra-only", "HasTestPostRAOnlyFus
// CHECK-PREDICATOR-NEXT: {{[[]}}{{[[]}}maybe_unused{{[]]}}{{[]]}} const MachineInstr *MI = &SecondMI;
// CHECK-PREDICATOR-NEXT: if (!(
// CHECK-PREDICATOR-NEXT: ( MI->getOpcode() == Test::Inst1 )
-// CHECK-PREDICATOR-NEXT: && MI->getOperand(0).getReg() == Test::X0
+// CHECK-PREDICATOR-NEXT: && (MI->getOperand(0).isReg() && MI->getOperand(0).getReg() == Test::X0)
// CHECK-PREDICATOR-NEXT: ))
// CHECK-PREDICATOR-NEXT: return false;
// CHECK-PREDICATOR-NEXT: }
@@ -227,7 +227,7 @@ def TestPostRAOnlyFusion: SimpleFusion<"test-postra-only", "HasTestPostRAOnlyFus
// CHECK-PREDICATOR-NEXT: {{[[]}}{{[[]}}maybe_unused{{[]]}}{{[]]}} const MachineInstr *MI = &SecondMI;
// CHECK-PREDICATOR-NEXT: if (!(
// CHECK-PREDICATOR-NEXT: ( MI->getOpcode() == Test::Inst1 )
-// CHECK-PREDICATOR-NEXT: && MI->getOperand(0).getReg() == Test::X0
+// CHECK-PREDICATOR-NEXT: && (MI->getOperand(0).isReg() && MI->getOperand(0).getReg() == Test::X0)
// CHECK-PREDICATOR-NEXT: ))
// CHECK-PREDICATOR-NEXT: return false;
// CHECK-PREDICATOR-NEXT: }
@@ -349,7 +349,7 @@ def TestPostRAOnlyFusion: SimpleFusion<"test-postra-only", "HasTestPostRAOnlyFus
// CHECK-PREDICATOR-NEXT: {{[[]}}{{[[]}}maybe_unused{{[]]}}{{[]]}} const MachineInstr *MI = &SecondMI;
// CHECK-PREDICATOR-NEXT: if (!(
// CHECK-PREDICATOR-NEXT: ( MI->getOpcode() == Test::Inst2 )
-// CHECK-PREDICATOR-NEXT: && MI->getOperand(0).getReg() == Test::X0
+// CHECK-PREDICATOR-NEXT: && (MI->getOperand(0).isReg() && MI->getOperand(0).getReg() == Test::X0)
// CHECK-PREDICATOR-NEXT: ))
// CHECK-PREDICATOR-NEXT: return false;
// CHECK-PREDICATOR-NEXT: }
diff --git a/llvm/utils/TableGen/Common/PredicateExpander.cpp b/llvm/utils/TableGen/Common/PredicateExpander.cpp
index c6d73f9c6721b..ac0942df3d112 100644
--- a/llvm/utils/TableGen/Common/PredicateExpander.cpp
+++ b/llvm/utils/TableGen/Common/PredicateExpander.cpp
@@ -88,30 +88,37 @@ void PredicateExpander::expandCheckRegOperand(raw_ostream &OS, int OpIndex,
StringRef FunctionMapper) {
assert(Reg->isSubClassOf("Register") && "Expected a register Record!");
+ OS << (shouldNegate() ? "!(" : "(");
+ OS << "MI" << (isByRef() ? "." : "->") << "getOperand(" << OpIndex
+ << ").isReg()";
+ OS << " && ";
if (!FunctionMapper.empty())
OS << FunctionMapper << "(";
OS << "MI" << (isByRef() ? "." : "->") << "getOperand(" << OpIndex
<< ").getReg()";
if (!FunctionMapper.empty())
OS << ")";
- OS << (shouldNegate() ? " != " : " == ");
+ OS << " == ";
const StringRef Str = Reg->getValueAsString("Namespace");
if (!Str.empty())
OS << Str << "::";
- OS << Reg->getName();
+ OS << Reg->getName() << ")";
}
void PredicateExpander::expandCheckRegOperandSimple(raw_ostream &OS,
int OpIndex,
StringRef FunctionMapper) {
- if (shouldNegate())
- OS << "!";
+ OS << (shouldNegate() ? "!(" : "(");
+ OS << "MI" << (isByRef() ? "." : "->") << "getOperand(" << OpIndex
+ << ").isReg()";
+ OS << " && ";
if (!FunctionMapper.empty())
OS << FunctionMapper << "(";
OS << "MI" << (isByRef() ? "." : "->") << "getOperand(" << OpIndex
<< ").getReg()";
if (!FunctionMapper.empty())
OS << ")";
+ OS << ")";
}
void PredicateExpander::expandCheckInvalidRegOperand(raw_ostream &OS,
``````````
</details>
https://github.com/llvm/llvm-project/pull/215230
More information about the llvm-commits
mailing list