[llvm] 6f72d28 - [TableGen][GISel] Don't copy dead def from a sub-instruction to the root (#121094)

[via llvm-commits]

Author: Sergei Barannikov
Date: 2024-12-26T08:36:35+03:00
New Revision: 6f72d28dd94c84b28275dbe61f5b2021df752e55

URL: https://github.com/llvm/llvm-project/commit/6f72d28dd94c84b28275dbe61f5b2021df752e55
DIFF: https://github.com/llvm/llvm-project/commit/6f72d28dd94c84b28275dbe61f5b2021df752e55.diff

LOG: [TableGen][GISel] Don't copy dead def from a sub-instruction to the root (#121094)

Sub-instruction can have a def with the same name as a def in a
top-level instruction.
Previously this could result in both defs copied to the instruction
being built.

Added: 
    llvm/test/TableGen/GlobalISelEmitter/dead-def.td

Modified: 
    llvm/utils/TableGen/GlobalISelEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/TableGen/GlobalISelEmitter/dead-def.td b/llvm/test/TableGen/GlobalISelEmitter/dead-def.td
new file mode 100644
index 00000000000000..a8597f1d840645
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelEmitter/dead-def.td
@@ -0,0 +1,27 @@
+// 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*/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_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),
+// 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,

diff  --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index 5038be7b24fbcb..9c945edfafe366 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -404,9 +404,10 @@ class GlobalISelEmitter final : public GlobalISelMatchTableExecutorEmitter {
   createInstructionRenderer(action_iterator InsertPt, RuleMatcher &M,
                             const TreePatternNode &Dst) const;
 
-  Expected<action_iterator> importExplicitDefRenderers(
-      action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
-      const TreePatternNode &Dst, unsigned Start = 0) const;
+  Expected<action_iterator>
+  importExplicitDefRenderers(action_iterator InsertPt, RuleMatcher &M,
+                             BuildMIAction &DstMIBuilder,
+                             const TreePatternNode &Dst, bool IsRoot) const;
 
   Expected<action_iterator>
   importExplicitUseRenderers(action_iterator InsertPt, RuleMatcher &M,
@@ -1369,7 +1370,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);
 
@@ -1398,8 +1400,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);
 
@@ -1440,16 +1442,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();