[llvm] [TableGen][GISel] Don't copy dead def from a sub-instruction to the root (PR #121094)
Sergei Barannikov via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 25 00:33:33 PST 2024
https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/121094
>From 903f279ec18e2592aaee7511db1ded0a829c2aa5 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Wed, 25 Dec 2024 11:17:14 +0300
Subject: [PATCH 1/2] add a test
---
.../TableGen/GlobalISelEmitter/dead-def.td | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
create mode 100644 llvm/test/TableGen/GlobalISelEmitter/dead-def.td
diff --git a/llvm/test/TableGen/GlobalISelEmitter/dead-def.td b/llvm/test/TableGen/GlobalISelEmitter/dead-def.td
new file mode 100644
index 00000000000000..50b5c6e012c129
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelEmitter/dead-def.td
@@ -0,0 +1,26 @@
+// RUN: llvm-tblgen -gen-global-isel -optimize-match-table=false \
+// RUN: -I %p/../../../include -I %p/../Common %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+include "GlobalISelEmitterCommon.td"
+
+// Check that $same_name from I2 isn't copied to the root instruction.
+
+def I1 : I<(outs GPR32:$same_name), (ins GPR32:$rs), []>;
+def I2 : I<(outs GPR32:$other_name, GPR32:$same_name), (ins GPR32:$rs), []>;
+
+def : Pat<(abs i32:$x), (I1 (I2 $x))>;
+
+// CHECK-LABEL: // (abs:{ *:[i32] } i32:{ *:[i32] }:$x) => (I1:{ *:[i32] } (I2:{ *:[i32] }:{ *:[i32] } ?:{ *:[i32] }:$x))
+// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(MyTarget::I2),
+// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/0, // DstI[same_name]
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // x
+// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/1,
+// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::I1),
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[same_name]
+// CHECK-NEXT: GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/0,
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 0,
+// CHECK-NEXT: GIR_EraseRootFromParent_Done,
>From d2a0e951d6682796595f680192ca1bef7881ced3 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Wed, 25 Dec 2024 11:20:27 +0300
Subject: [PATCH 2/2] fix the bug
---
llvm/test/TableGen/GlobalISelEmitter/dead-def.td | 3 ++-
llvm/utils/TableGen/GlobalISelEmitter.cpp | 15 ++++++++-------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/llvm/test/TableGen/GlobalISelEmitter/dead-def.td b/llvm/test/TableGen/GlobalISelEmitter/dead-def.td
index 50b5c6e012c129..a8597f1d840645 100644
--- a/llvm/test/TableGen/GlobalISelEmitter/dead-def.td
+++ b/llvm/test/TableGen/GlobalISelEmitter/dead-def.td
@@ -12,10 +12,11 @@ def I2 : I<(outs GPR32:$other_name, GPR32:$same_name), (ins GPR32:$rs), []>;
def : Pat<(abs i32:$x), (I1 (I2 $x))>;
// CHECK-LABEL: // (abs:{ *:[i32] } i32:{ *:[i32] }:$x) => (I1:{ *:[i32] } (I2:{ *:[i32] }:{ *:[i32] } ?:{ *:[i32] }:$x))
+// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/1, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(MyTarget::I2),
// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
-// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/0, // DstI[same_name]
+// CHECK-NEXT: GIR_AddTempRegister, /*InsnID*/1, /*TempRegID*/1, /*TempRegFlags*/GIMT_Encode2(RegState::Define|RegState::Dead),
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // x
// CHECK-NEXT: GIR_ConstrainSelectedInstOperands, /*InsnID*/1,
// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::I1),
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index 4250b57581f63e..4d67aed0ae1f4a 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -406,7 +406,7 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
Expected<action_iterator> importExplicitDefRenderers(
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
- const TreePatternNode &Dst, unsigned Start = 0) const;
+ const TreePatternNode &Dst, bool IsRoot) const;
Expected<action_iterator>
importExplicitUseRenderers(action_iterator InsertPt, RuleMatcher &M,
@@ -1375,7 +1375,8 @@ Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
CopyToPhysRegMIBuilder.addRenderer<CopyPhysRegRenderer>(PhysInput.first);
}
- if (auto Error = importExplicitDefRenderers(InsertPt, M, DstMIBuilder, Dst)
+ if (auto Error = importExplicitDefRenderers(InsertPt, M, DstMIBuilder, Dst,
+ /*IsRoot=*/true)
.takeError())
return std::move(Error);
@@ -1404,8 +1405,8 @@ GlobalISelEmitter::createAndImportSubInstructionRenderer(
DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true);
// Handle additional (ignored) results.
- InsertPtOrError = importExplicitDefRenderers(std::prev(*InsertPtOrError), M,
- DstMIBuilder, Dst, /*Start=*/1);
+ InsertPtOrError = importExplicitDefRenderers(
+ std::prev(*InsertPtOrError), M, DstMIBuilder, Dst, /*IsRoot=*/false);
if (auto Error = InsertPtOrError.takeError())
return std::move(Error);
@@ -1446,16 +1447,16 @@ GlobalISelEmitter::createInstructionRenderer(action_iterator InsertPt,
Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
- const TreePatternNode &Dst, unsigned Start) const {
+ const TreePatternNode &Dst, bool IsRoot) const {
const CodeGenInstruction *DstI = DstMIBuilder.getCGI();
// Process explicit defs. The caller may have already handled the first def.
- for (unsigned I = Start, E = DstI->Operands.NumDefs; I != E; ++I) {
+ for (unsigned I = IsRoot ? 0 : 1, E = DstI->Operands.NumDefs; I != E; ++I) {
const CGIOperandList::OperandInfo &OpInfo = DstI->Operands[I];
std::string OpName = getMangledRootDefName(OpInfo.Name);
// If the def is used in the source DAG, forward it.
- if (M.hasOperand(OpName)) {
+ if (IsRoot && M.hasOperand(OpName)) {
// CopyRenderer saves a StringRef, so cannot pass OpName itself -
// let's use a string with an appropriate lifetime.
StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
More information about the llvm-commits
mailing list