[llvm] [TableGen][GISel] Learn to import patterns with optional defs (PR #120470)

Sergey Barannikov via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 20 17:21:10 PST 2024


https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/120470

>From 4a8b23d6e527b056cddf2f2ec288397f17169cbc Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <s.barannikov at module.ru>
Date: Wed, 18 Dec 2024 22:20:30 +0300
Subject: [PATCH 1/2] [TableGen][GISel] Learn to import patterns with optional
 defs

---
 .../GlobalISelEmitter-optional-def.td         | 37 +++++++++++++++++++
 .../GlobalISel/GlobalISelMatchTable.cpp       |  9 +++--
 .../Common/GlobalISel/GlobalISelMatchTable.h  |  6 ++-
 llvm/utils/TableGen/GlobalISelEmitter.cpp     | 24 +++++++++---
 4 files changed, 65 insertions(+), 11 deletions(-)
 create mode 100644 llvm/test/TableGen/GlobalISelEmitter-optional-def.td

diff --git a/llvm/test/TableGen/GlobalISelEmitter-optional-def.td b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td
new file mode 100644
index 00000000000000..cfb105cd0218bc
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td
@@ -0,0 +1,37 @@
+// RUN: llvm-tblgen -gen-global-isel -I %p/../../include -I %p/Common < %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+include "GlobalISelEmitterCommon.td"
+
+def cc_out : OptionalDefOperand<i32, (ops GPR8), (ops (i8 zero_reg))>;
+def s_cc_out : OptionalDefOperand<i32, (ops GPR8, FPR32), (ops (i8 B0), F0)>;
+
+// CHECK-LABEL: // (add:{ *:[i32] } i32:{ *:[i32] }:$rs1, i32:{ *:[i32] }:$rs2)  =>  (tst2:{ *:[i32] } i32:{ *:[i32] }:$rs1, i32:{ *:[i32] }:$rs2)
+// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::tst2),
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[rd]
+// CHECK-NEXT: GIR_AddRegister, /*InsnID*/0, GIMT_Encode2(MyTarget::B0), /*AddRegisterRegFlags*/GIMT_Encode2(RegState::Define | RegState::Dead),
+// CHECK-NEXT: GIR_AddRegister, /*InsnID*/0, GIMT_Encode2(MyTarget::F0), /*AddRegisterRegFlags*/GIMT_Encode2(RegState::Define | RegState::Dead),
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/1, // rs1
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/2, // rs2
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 1,
+// CHECK-NEXT: GIR_EraseRootFromParent_Done,
+
+// CHECK-LABEL: // (imm:{ *:[i32] }):$imm  =>  (tst1:{ *:[i32] } (imm:{ *:[i32] }):$imm)
+// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::tst1),
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[rd]
+// CHECK-NEXT: GIR_AddRegister, /*InsnID*/0, GIMT_Encode2(MyTarget::NoRegister), /*AddRegisterRegFlags*/GIMT_Encode2(RegState::Define | RegState::Dead),
+// CHECK-NEXT: GIR_CopyConstantAsSImm, /*NewInsnID*/0, /*OldInsnID*/0, // imm
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 0,
+// CHECK-NEXT: GIR_EraseRootFromParent_Done,
+
+def tst1 : I<(outs GPR32:$rd, cc_out:$s), (ins i32imm:$imm),
+             [(set GPR32:$rd, imm:$imm)]>;
+
+def tst2 : I<(outs GPR32:$rd, s_cc_out:$s), (ins GPR32:$rs1, GPR32:$rs2),
+             [(set GPR32:$rd, (add i32:$rs1, i32:$rs2))]>;
+
+// TODO: There should be more tests, but any attempt to write something
+//   more complex results in tablegen crashing somewhere in
+//   TreePatternNode::UpdateNodeType.
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
index 15ec7e17130de4..619e7a4790c88b 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
@@ -1993,10 +1993,13 @@ void AddRegisterRenderer::emitRenderOpcodes(MatchTable &Table,
   // TODO: This is encoded as a 64-bit element, but only 16 or 32-bits are
   // really needed for a physical register reference. We can pack the
   // register and flags in a single field.
-  if (IsDef)
-    Table << MatchTable::NamedValue(2, "RegState::Define");
-  else
+  if (IsDef) {
+    Table << MatchTable::NamedValue(
+        2, IsDead ? "RegState::Define | RegState::Dead" : "RegState::Define");
+  } else {
+    assert(!IsDead && "A use cannot be dead");
     Table << MatchTable::IntValue(2, 0);
+  }
   Table << MatchTable::LineBreak;
 }
 
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
index 00fe073057c5c9..48ce71be677c08 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
@@ -2091,13 +2091,15 @@ class AddRegisterRenderer : public OperandRenderer {
   unsigned InsnID;
   const Record *RegisterDef;
   bool IsDef;
+  bool IsDead;
   const CodeGenTarget &Target;
 
 public:
   AddRegisterRenderer(unsigned InsnID, const CodeGenTarget &Target,
-                      const Record *RegisterDef, bool IsDef = false)
+                      const Record *RegisterDef, bool IsDef = false,
+                      bool IsDead = false)
       : OperandRenderer(OR_Register), InsnID(InsnID), RegisterDef(RegisterDef),
-        IsDef(IsDef), Target(Target) {}
+        IsDef(IsDef), IsDead(IsDead), Target(Target) {}
 
   static bool classof(const OperandRenderer *R) {
     return R->getKind() == OR_Register;
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index ca05c10ed81aed..ef9bf154767d5e 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -1451,14 +1451,10 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
     const TreePatternNode &Dst, unsigned Start) {
   const CodeGenInstruction *DstI = DstMIBuilder.getCGI();
 
-  // Some instructions have multiple defs, but are missing a type entry
-  // (e.g. s_cc_out operands).
-  if (Dst.getExtTypes().size() < DstI->Operands.NumDefs)
-    return failedImport("unhandled discarded def");
-
   // Process explicit defs. The caller may have already handled the first def.
   for (unsigned I = Start, E = DstI->Operands.NumDefs; I != E; ++I) {
-    std::string OpName = getMangledRootDefName(DstI->Operands[I].Name);
+    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)) {
@@ -1469,6 +1465,22 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
       continue;
     }
 
+    // A discarded explicit def may be an optional physical register.
+    // If this is the case, add the default register and mark it as dead.
+    if (OpInfo.Rec->isSubClassOf("OptionalDefOperand")) {
+      for (const TreePatternNode &DefaultOp :
+           make_pointee_range(CGP.getDefaultOperand(OpInfo.Rec).DefaultOps)) {
+        const Record *Reg = cast<DefInit>(DefaultOp.getLeafValue())->getDef();
+
+        if (!Reg->isSubClassOf("Register") && Reg->getName() != "zero_reg")
+          return failedImport("optional def is not a register");
+
+        DstMIBuilder.addRenderer<AddRegisterRenderer>(
+            Target, Reg, /*IsDef=*/true, /*IsDead=*/true);
+      }
+      continue;
+    }
+
     // The def is discarded, create a dead virtual register for it.
     const TypeSetByHwMode &ExtTy = Dst.getExtType(I);
     if (!ExtTy.isMachineValueType())

>From 29baa13fd4f569ed68ffc81b317a5d9348c69c75 Mon Sep 17 00:00:00 2001
From: Sergey Barannikov <barannikov88 at gmail.com>
Date: Sat, 21 Dec 2024 04:16:37 +0300
Subject: [PATCH 2/2] Add a few negative tests

---
 .../GlobalISelEmitter-optional-def.td         | 21 ++++++++++++++++++-
 llvm/utils/TableGen/GlobalISelEmitter.cpp     |  9 +++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/llvm/test/TableGen/GlobalISelEmitter-optional-def.td b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td
index cfb105cd0218bc..def4a0447fe53f 100644
--- a/llvm/test/TableGen/GlobalISelEmitter-optional-def.td
+++ b/llvm/test/TableGen/GlobalISelEmitter-optional-def.td
@@ -1,4 +1,6 @@
-// RUN: llvm-tblgen -gen-global-isel -I %p/../../include -I %p/Common < %s | FileCheck %s
+// RUN: llvm-tblgen -gen-global-isel  -warn-on-skipped-patterns \
+// RUN:   -I %p/../../include -I %p/Common %s 2> %t | FileCheck %s
+// RUN: FileCheck -DFILE=%s -check-prefix=ERR %s < %t
 
 include "llvm/Target/Target.td"
 include "GlobalISelEmitterCommon.td"
@@ -35,3 +37,20 @@ def tst2 : I<(outs GPR32:$rd, s_cc_out:$s), (ins GPR32:$rs1, GPR32:$rs2),
 // TODO: There should be more tests, but any attempt to write something
 //   more complex results in tablegen crashing somewhere in
 //   TreePatternNode::UpdateNodeType.
+
+
+def not_leaf : OptionalDefOperand<i32, (ops GPR8), (ops (i8 imm))>;
+def not_rec  : OptionalDefOperand<i32, (ops GPR8), (ops (i8 0))>;
+def not_reg  : OptionalDefOperand<i32, (ops GPR8), (ops GPR8)>;
+
+// ERR: [[#@LINE+1]]:5: warning: Skipped pattern: optional def is not a leaf
+def tst_not_leaf : I<(outs GPR32:$rd, not_leaf:$s), (ins i32imm:$imm),
+                     [(set GPR32:$rd, imm:$imm)]>;
+
+// ERR: [[#@LINE+1]]:5: warning: Skipped pattern: optional def is not a record
+def tst_not_rec : I<(outs GPR32:$rd, not_rec:$s), (ins i32imm:$imm),
+                    [(set GPR32:$rd, imm:$imm)]>;
+
+// ERR: [[#@LINE+1]]:5: warning: Skipped pattern: optional def is not a register
+def tst_not_reg : I<(outs GPR32:$rd, not_reg:$s), (ins i32imm:$imm),
+                    [(set GPR32:$rd, imm:$imm)]>;
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index ef9bf154767d5e..3f504b73465d21 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -1470,8 +1470,15 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
     if (OpInfo.Rec->isSubClassOf("OptionalDefOperand")) {
       for (const TreePatternNode &DefaultOp :
            make_pointee_range(CGP.getDefaultOperand(OpInfo.Rec).DefaultOps)) {
-        const Record *Reg = cast<DefInit>(DefaultOp.getLeafValue())->getDef();
+        // TODO: Do these checks in ParseDefaultOperands.
+        if (!DefaultOp.isLeaf())
+          return failedImport("optional def is not a leaf");
 
+        const auto *RegDI = dyn_cast<DefInit>(DefaultOp.getLeafValue());
+        if (!RegDI)
+          return failedImport("optional def is not a record");
+
+        const Record *Reg = RegDI->getDef();
         if (!Reg->isSubClassOf("Register") && Reg->getName() != "zero_reg")
           return failedImport("optional def is not a register");
 



More information about the llvm-commits mailing list