[llvm] [llvm][TableGen] Allow physical registers in patterns for GlobalISel emitter (PR #113127)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 26 18:56:00 PDT 2025
https://github.com/JL2210 updated https://github.com/llvm/llvm-project/pull/113127
>From a99b8b120fca3a28f7723d493752205d35c1de5d Mon Sep 17 00:00:00 2001
From: James R Larrowe <larrowe.semaj11 at gmail.com>
Date: Sun, 20 Oct 2024 22:19:11 -0400
Subject: [PATCH 1/4] Allow physical registers in patterns
Most of the heavy lifting was already done in
importExplicitDefRenderers, so I just added an operand for the
physical register defs.
The logic introduced in #112673 was indeed subtly broken; it was
only supposed to count the physical register defs present in the
pattern rather than all physical register defs.
Remove importImplicitDefRenderers since it was unimplemented
anyway, and I don't see a way that it could be implemented
satisfactorily. Also, the name was rather misleading; it should've
been called something like importPhysRegDefRenderers
---
llvm/utils/TableGen/GlobalISelEmitter.cpp | 95 ++++++++++++-----------
1 file changed, 50 insertions(+), 45 deletions(-)
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index e866bd983e04e..db8bf826cc5a7 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -398,7 +398,8 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
Expected<BuildMIAction &> createAndImportInstructionRenderer(
RuleMatcher &M, InstructionMatcher &InsnMatcher,
- const TreePatternNode &Src, const TreePatternNode &Dst);
+ const TreePatternNode &Src, const TreePatternNode &Dst,
+ ArrayRef<const Record *> DstPhysDefs);
Expected<action_iterator> createAndImportSubInstructionRenderer(
action_iterator InsertPt, RuleMatcher &M, const TreePatternNode &Dst,
const TreePatternNode &Src, unsigned TempReg);
@@ -406,11 +407,10 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
createInstructionRenderer(action_iterator InsertPt, RuleMatcher &M,
const TreePatternNode &Dst);
- Expected<action_iterator>
- importExplicitDefRenderers(action_iterator InsertPt, RuleMatcher &M,
- BuildMIAction &DstMIBuilder,
- const TreePatternNode &Src,
- const TreePatternNode &Dst, unsigned Start = 0);
+ Expected<action_iterator> importExplicitDefRenderers(
+ action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
+ const TreePatternNode &Src, const TreePatternNode &Dst,
+ ArrayRef<const Record *> DstPhysDefs = {}, unsigned Start = 0);
Expected<action_iterator> importExplicitUseRenderers(
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
@@ -421,8 +421,6 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
Error importDefaultOperandRenderers(action_iterator InsertPt, RuleMatcher &M,
BuildMIAction &DstMIBuilder,
const DAGDefaultOperand &DefaultOp) const;
- Error importImplicitDefRenderers(BuildMIAction &DstMIBuilder,
- ArrayRef<const Record *> ImplicitDefs) const;
/// Analyze pattern \p P, returning a matcher for it if possible.
/// Otherwise, return an Error explaining why we don't support it.
@@ -1348,7 +1346,7 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitUseRenderer(
Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
RuleMatcher &M, InstructionMatcher &InsnMatcher, const TreePatternNode &Src,
- const TreePatternNode &Dst) {
+ const TreePatternNode &Dst, ArrayRef<const Record *> DstPhysDefs) {
auto InsertPtOrError = createInstructionRenderer(M.actions_end(), M, Dst);
if (auto Error = InsertPtOrError.takeError())
return std::move(Error);
@@ -1367,9 +1365,9 @@ Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
CopyToPhysRegMIBuilder.addRenderer<CopyPhysRegRenderer>(PhysInput.first);
}
- if (auto Error =
- importExplicitDefRenderers(InsertPt, M, DstMIBuilder, Src, Dst)
- .takeError())
+ if (auto Error = importExplicitDefRenderers(InsertPt, M, DstMIBuilder, Src,
+ Dst, DstPhysDefs)
+ .takeError())
return std::move(Error);
if (auto Error =
@@ -1399,8 +1397,9 @@ GlobalISelEmitter::createAndImportSubInstructionRenderer(
// Handle additional (ignored) results.
if (DstMIBuilder.getCGI()->Operands.NumDefs > 1) {
- InsertPtOrError = importExplicitDefRenderers(
- std::prev(*InsertPtOrError), M, DstMIBuilder, Src, Dst, /*Start=*/1);
+ InsertPtOrError =
+ importExplicitDefRenderers(std::prev(*InsertPtOrError), M, DstMIBuilder,
+ Src, Dst, /*DstPhysDefs=*/{}, /*Start=*/1);
if (auto Error = InsertPtOrError.takeError())
return std::move(Error);
}
@@ -1533,19 +1532,29 @@ Expected<action_iterator> GlobalISelEmitter::createInstructionRenderer(
Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
- const TreePatternNode &Src, const TreePatternNode &Dst, unsigned Start) {
+ const TreePatternNode &Src, const TreePatternNode &Dst,
+ ArrayRef<const Record *> DstPhysDefs, unsigned Start) {
const CodeGenInstruction *DstI = DstMIBuilder.getCGI();
const unsigned SrcNumDefs = Src.getExtTypes().size();
- const unsigned DstNumDefs = DstI->Operands.NumDefs;
+ const unsigned DstNumVirtDefs = DstI->Operands.NumDefs,
+ DstNumDefs = DstNumVirtDefs + DstPhysDefs.size();
if (DstNumDefs == 0)
return InsertPt;
for (unsigned I = Start; I < SrcNumDefs; ++I) {
- std::string OpName = getMangledRootDefName(DstI->Operands[I].Name);
- // CopyRenderer saves a StringRef, so cannot pass OpName itself -
- // let's use a string with an appropriate lifetime.
- StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
- DstMIBuilder.addRenderer<CopyRenderer>(PermanentRef);
+ if (I < DstNumVirtDefs) {
+ std::string OpName = getMangledRootDefName(DstI->Operands[I].Name);
+ // CopyRenderer saves a StringRef, so cannot pass OpName itself -
+ // let's use a string with an appropriate lifetime.
+ StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
+ DstMIBuilder.addRenderer<CopyRenderer>(PermanentRef);
+ } else if (I < DstNumDefs) {
+ const auto *PhysReg = DstPhysDefs[I - DstNumVirtDefs];
+ DstMIBuilder.addRenderer<CopyPhysRegRenderer>(PhysReg);
+ } else {
+ return failedImport("number of defs in src exceeds number of implicit "
+ "and explicit defs in dst");
+ }
}
// Some instructions have multiple defs, but are missing a type entry
@@ -1788,13 +1797,6 @@ Error GlobalISelEmitter::importDefaultOperandRenderers(
return Error::success();
}
-Error GlobalISelEmitter::importImplicitDefRenderers(
- BuildMIAction &DstMIBuilder, ArrayRef<const Record *> ImplicitDefs) const {
- if (!ImplicitDefs.empty())
- return failedImport("Pattern defines a physical register");
- return Error::success();
-}
-
std::optional<const CodeGenRegisterClass *>
GlobalISelEmitter::getRegClassFromLeaf(const TreePatternNode &Leaf) {
assert(Leaf.isLeaf() && "Expected leaf?");
@@ -2022,11 +2024,9 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
auto &DstI = Target.getInstruction(DstOp);
StringRef DstIName = DstI.TheDef->getName();
-
- // Count both implicit and explicit defs in the dst instruction.
- // This avoids errors importing patterns that have inherent implicit defs.
- unsigned DstExpDefs = DstI.Operands.NumDefs,
- DstNumDefs = DstI.ImplicitDefs.size() + DstExpDefs,
+ const auto &DstPhysDefs = P.getDstRegs();
+ unsigned DstNumVirtDefs = DstI.Operands.NumDefs,
+ DstNumDefs = DstNumVirtDefs + DstPhysDefs.size(),
SrcNumDefs = Src.getExtTypes().size();
if (DstNumDefs < SrcNumDefs) {
if (DstNumDefs != 0)
@@ -2048,13 +2048,13 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
// The root of the match also has constraints on the register bank so that it
// matches the result instruction.
unsigned OpIdx = 0;
- unsigned N = std::min(DstExpDefs, SrcNumDefs);
+ unsigned N = std::min(DstNumDefs, SrcNumDefs);
for (unsigned I = 0; I < N; ++I) {
const TypeSetByHwMode &VTy = Src.getExtType(I);
- const auto &DstIOperand = DstI.Operands[OpIdx];
PointerUnion<const Record *, const CodeGenRegisterClass *> MatchedRC =
- DstIOperand.Rec;
+ OpIdx < DstNumVirtDefs ? DstI.Operands[OpIdx].Rec
+ : DstPhysDefs[OpIdx - DstNumVirtDefs];
if (DstIName == "COPY_TO_REGCLASS") {
MatchedRC = getInitValueAsRegClass(Dst.getChild(1).getLeafValue());
@@ -2092,7 +2092,13 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
MatchedRC = *MaybeRegClass;
} else if (MatchedRC.get<const Record *>()->isSubClassOf("RegisterOperand"))
MatchedRC = MatchedRC.get<const Record *>()->getValueAsDef("RegClass");
- else if (!MatchedRC.get<const Record *>()->isSubClassOf("RegisterClass"))
+ else if (MatchedRC.get<const Record *>()->isSubClassOf("Register")) {
+ auto MaybeRegClass =
+ CGRegs.getRegClassForRegister(MatchedRC.get<const Record *>());
+ if (!MaybeRegClass)
+ return failedImport("Cannot infer register class for register");
+ MatchedRC = MaybeRegClass;
+ } else if (!MatchedRC.get<const Record *>()->isSubClassOf("RegisterClass"))
return failedImport("Dst MI def isn't a register class" + to_string(Dst));
OperandMatcher &OM = InsnMatcher.getOperand(OpIdx);
@@ -2100,8 +2106,12 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
// those used in pattern's source and destination DAGs, so mangle the
// former to prevent implicitly adding unexpected
// GIM_CheckIsSameOperand predicates by the defineOperand method.
- OM.setSymbolicName(getMangledRootDefName(DstIOperand.Name));
- M.defineOperand(OM.getSymbolicName(), OM);
+ if (OpIdx < DstNumVirtDefs) {
+ OM.setSymbolicName(getMangledRootDefName(DstI.Operands[OpIdx].Name));
+ M.defineOperand(OM.getSymbolicName(), OM);
+ } else {
+ M.definePhysRegOperand(DstPhysDefs[OpIdx - DstNumVirtDefs], OM);
+ }
if (MatchedRC.is<const Record *>())
MatchedRC = &Target.getRegisterClass(MatchedRC.get<const Record *>());
OM.addPredicate<RegisterBankOperandMatcher>(
@@ -2110,16 +2120,11 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
}
auto DstMIBuilderOrError =
- createAndImportInstructionRenderer(M, InsnMatcher, Src, Dst);
+ createAndImportInstructionRenderer(M, InsnMatcher, Src, Dst, DstPhysDefs);
if (auto Error = DstMIBuilderOrError.takeError())
return std::move(Error);
BuildMIAction &DstMIBuilder = DstMIBuilderOrError.get();
- // Render the implicit defs.
- // These are only added to the root of the result.
- if (auto Error = importImplicitDefRenderers(DstMIBuilder, P.getDstRegs()))
- return std::move(Error);
-
DstMIBuilder.chooseInsnToMutate(M);
// Constrain the registers to classes. This is normally derived from the
>From bef517e3c17f73ceec26b58310a708511c048f46 Mon Sep 17 00:00:00 2001
From: James R Larrowe <larrowe.semaj11 at gmail.com>
Date: Mon, 21 Oct 2024 01:03:59 -0400
Subject: [PATCH 2/4] Add test for implicit defs
thanks @s-barannikov!
---
.../Common/GlobalISelEmitterCommon.td | 3 +-
.../GlobalISelEmitter-implicit-defs.td | 28 +++++++++++++++++--
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td b/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td
index 8f11fee375184..8f625811eb9b2 100644
--- a/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td
+++ b/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td
@@ -7,7 +7,8 @@ class MyTargetGenericInstruction : GenericInstruction {
}
def R0 : Register<"r0"> { let Namespace = "MyTarget"; }
-def GPR32 : RegisterClass<"MyTarget", [i32], 32, (add R0)>;
+def R1 : Register<"r1"> { let Namespace = "MyTarget"; }
+def GPR32 : RegisterClass<"MyTarget", [i32], 32, (add R0, R1)>;
def GPR32Op : RegisterOperand<GPR32>;
def F0 : Register<"f0"> { let Namespace = "MyTarget"; }
def FPR32 : RegisterClass<"MyTarget", [f32], 32, (add F0)>;
diff --git a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
index 79af1a336f289..36d0090b7ab16 100644
--- a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
+++ b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
@@ -3,10 +3,34 @@
include "llvm/Target/Target.td"
include "GlobalISelEmitterCommon.td"
-// CHECK: Skipped pattern: Pattern defines a physical register
let Uses = [B0], Defs = [B0] in
def tst1 : I<(outs), (ins), [(set B0, (add B0, 1))]>;
-// CHECK: Skipped pattern: Src pattern result has 1 def(s) without the HasNoUse predicate set to true but Dst MI has no def
+// CHECK: Skipped pattern: unhandled discarded def
let Uses = [B0] in
def tst2 : I<(outs), (ins), [(set B0, (add B0, 1))]>;
+
+// test courtesy @s-barannikov
+def SDTBinOpWithFlagsOut : SDTypeProfile<2, 2, [
+ SDTCisInt<0>, // result
+ SDTCisVT<1, i32>, // out flags
+ SDTCisSameAs<2, 0>, // lhs
+ SDTCisSameAs<3, 0> // rhs
+]>;
+
+def my_sub : SDNode<"MyTargetISD::SUB", SDTBinOpWithFlagsOut>;
+def my_ineg : PatFrag<(ops node:$val), (my_sub 0, node:$val)>;
+
+let Defs = [R1], Constraints = "$rd = $rs2" in
+def tst3 : I<(outs GPR32:$rd), (ins GPR32:$rs2), []>;
+
+// CHECK: Skipped pattern: Src pattern result has more defs than dst MI (2 def(s) vs 1 def(s))
+def : Pat<(my_ineg i32:$val), (tst3 i32:$val)>;
+
+def G_MY_SUB : GenericInstruction {
+ let Namespace = "MyTarget";
+ let OutOperandList = (outs type0:$dst, type1:$flags_out);
+ let InOperandList = (ins type0:$lhs, type0:$rhs);
+}
+
+def : GINodeEquiv<G_MY_SUB, my_sub>;
>From fe3392c436e8c599e611d6ae9cf9a3278a39b322 Mon Sep 17 00:00:00 2001
From: James R Larrowe <larrowe.semaj11 at gmail.com>
Date: Tue, 22 Oct 2024 16:57:02 -0400
Subject: [PATCH 3/4] fixup! Allow physical registers in patterns
don't use auto
---
llvm/utils/TableGen/GlobalISelEmitter.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index db8bf826cc5a7..44129dacfce07 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -399,7 +399,7 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
Expected<BuildMIAction &> createAndImportInstructionRenderer(
RuleMatcher &M, InstructionMatcher &InsnMatcher,
const TreePatternNode &Src, const TreePatternNode &Dst,
- ArrayRef<const Record *> DstPhysDefs);
+ const ArrayRef<const Record *> DstPhysDefs);
Expected<action_iterator> createAndImportSubInstructionRenderer(
action_iterator InsertPt, RuleMatcher &M, const TreePatternNode &Dst,
const TreePatternNode &Src, unsigned TempReg);
@@ -2024,7 +2024,7 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
auto &DstI = Target.getInstruction(DstOp);
StringRef DstIName = DstI.TheDef->getName();
- const auto &DstPhysDefs = P.getDstRegs();
+ ArrayRef<const Record *> DstPhysDefs = P.getDstRegs();
unsigned DstNumVirtDefs = DstI.Operands.NumDefs,
DstNumDefs = DstNumVirtDefs + DstPhysDefs.size(),
SrcNumDefs = Src.getExtTypes().size();
>From a7c4df036e245de4aa11c12679a7b8c9bc77a31b Mon Sep 17 00:00:00 2001
From: James R Larrowe <larrowe.semaj11 at gmail.com>
Date: Tue, 22 Oct 2024 16:57:23 -0400
Subject: [PATCH 4/4] fixup! Add test for implicit defs
add testcase for missing register class
---
.../test/TableGen/Common/GlobalISelEmitterCommon.td | 1 +
.../TableGen/GlobalISelEmitter-implicit-defs.td | 13 ++++++++-----
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td b/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td
index 8f625811eb9b2..82868dce8f679 100644
--- a/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td
+++ b/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td
@@ -17,6 +17,7 @@ def B0 : Register<"b0"> { let Namespace = "MyTarget"; }
def GPR8 : RegisterClass<"MyTarget", [i8], 8, (add B0)>;
def GPR8Op : RegisterOperand<GPR8>;
+def NoRC : Register<"norc">;
def V0 : Register<"v0"> { let Namespace = "MyTarget"; }
def VecReg128 : RegisterClass<"MyTarget", [v4i32], 128, (add V0)>;
diff --git a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
index 36d0090b7ab16..baa57b856aad2 100644
--- a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
+++ b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
@@ -4,13 +4,16 @@ include "llvm/Target/Target.td"
include "GlobalISelEmitterCommon.td"
let Uses = [B0], Defs = [B0] in
-def tst1 : I<(outs), (ins), [(set B0, (add B0, 1))]>;
+def pass : I<(outs), (ins), [(set B0, (add B0, 1))]>;
// CHECK: Skipped pattern: unhandled discarded def
let Uses = [B0] in
-def tst2 : I<(outs), (ins), [(set B0, (add B0, 1))]>;
+def discarded : I<(outs), (ins), [(set B0, (add B0, 1))]>;
+
+// CHECK: Cannot infer register class for register
+let Defs = [NoRC] in
+def no_regclass : I<(outs), (ins), [(set NoRC, (add NoRC, 1))]>;
-// test courtesy @s-barannikov
def SDTBinOpWithFlagsOut : SDTypeProfile<2, 2, [
SDTCisInt<0>, // result
SDTCisVT<1, i32>, // out flags
@@ -22,10 +25,10 @@ def my_sub : SDNode<"MyTargetISD::SUB", SDTBinOpWithFlagsOut>;
def my_ineg : PatFrag<(ops node:$val), (my_sub 0, node:$val)>;
let Defs = [R1], Constraints = "$rd = $rs2" in
-def tst3 : I<(outs GPR32:$rd), (ins GPR32:$rs2), []>;
+def extra_def : I<(outs GPR32:$rd), (ins GPR32:$rs2), []>;
// CHECK: Skipped pattern: Src pattern result has more defs than dst MI (2 def(s) vs 1 def(s))
-def : Pat<(my_ineg i32:$val), (tst3 i32:$val)>;
+def : Pat<(my_ineg i32:$val), (extra_def i32:$val)>;
def G_MY_SUB : GenericInstruction {
let Namespace = "MyTarget";
More information about the llvm-commits
mailing list