[llvm] [WIP][TableGen][GISel] Learn to import patterns with physreg defs (PR #120343)

Sergei Barannikov via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 20 21:34:02 PST 2024


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

>From 5373dbe2a4d98287d6530231ab3273e7aceab34a Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Tue, 17 Dec 2024 19:39:15 +0300
Subject: [PATCH] [TableGen][GISel] Learn to import patterns with physreg defs

---
 .../test/CodeGen/X86/GlobalISel/mul-scalar.ll |  9 +--
 .../Common/GlobalISelEmitterCommon.td         |  3 +-
 .../GlobalISelEmitter-implicit-defs.td        | 59 +++++++++++++--
 llvm/utils/TableGen/GlobalISelEmitter.cpp     | 72 ++++++++++---------
 4 files changed, 99 insertions(+), 44 deletions(-)

diff --git a/llvm/test/CodeGen/X86/GlobalISel/mul-scalar.ll b/llvm/test/CodeGen/X86/GlobalISel/mul-scalar.ll
index f401f45a06f6a7..3196668c70d8ec 100644
--- a/llvm/test/CodeGen/X86/GlobalISel/mul-scalar.ll
+++ b/llvm/test/CodeGen/X86/GlobalISel/mul-scalar.ll
@@ -5,16 +5,17 @@
 define i8 @test_mul_i8(i8 %arg1, i8 %arg2) nounwind {
 ; X64-LABEL: test_mul_i8:
 ; X64:       # %bb.0:
-; X64-NEXT:    movsbl %dil, %eax
-; X64-NEXT:    imulb %sil
+; X64-NEXT:    movl %edi, %eax
+; X64-NEXT:    # kill: def $al killed $al killed $eax
+; X64-NEXT:    mulb %sil
 ; X64-NEXT:    retq
 ;
 ; X86-LABEL: test_mul_i8:
 ; X86:       # %bb.0:
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; X86-NEXT:    movl {{[0-9]+}}(%esp), %ecx
-; X86-NEXT:    cbtw
-; X86-NEXT:    imulb %cl
+; X86-NEXT:    # kill: def $al killed $al killed $eax
+; X86-NEXT:    mulb %cl
 ; X86-NEXT:    retl
   %ret = mul i8 %arg1, %arg2
   ret i8 %ret
diff --git a/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td b/llvm/test/TableGen/Common/GlobalISelEmitterCommon.td
index 8f11fee3751844..cfcaf3c76bbf8a 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<"r0"> { 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 79af1a336f2890..bc9d7a31e203c1 100644
--- a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
+++ b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
@@ -1,12 +1,57 @@
-// RUN: llvm-tblgen -gen-global-isel -warn-on-skipped-patterns -I %p/../../include -I %p/Common %s -o /dev/null 2>&1 < %s | FileCheck %s --implicit-check-not="Skipped pattern"
+// RUN: llvm-tblgen -gen-global-isel -I %p/../../include -I %p/Common %s | FileCheck %s
 
 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))]>;
+let Defs = [R0, R1] in
+def tst1 : I<(outs), (ins), [(set R0, (get_fpenv))]>;
 
-// CHECK: Skipped pattern: Src pattern result has 1 def(s) without the HasNoUse predicate set to true but Dst MI has no def
-let Uses = [B0] in
-def tst2 : I<(outs), (ins), [(set B0, (add B0, 1))]>;
+let Defs = [R0, R1] in
+def tst2 : I<(outs GPR32:$rd), (ins GPR32:$rs1, GPR32:$rs2),
+             [(set GPR32:$rd, R0, (udivrem i32:$rs1, i32:$rs2))]>;
+
+def : Pat<(sdiv i32:$rs1, i32:$rs2), (tst2 $rs1, $rs2)>;
+def : Pat<(sdivrem i32:$rs1, i32:$rs2), (tst2 $rs1, $rs2)>;
+
+// CHECK-LABEL: // (sdiv:{ *:[i32] } i32:{ *:[i32] }:$rs1, i32:{ *:[i32] }:$rs2)  =>  (tst2:{ *:[i32] }:{ *:[i32] } ?:{ *:[i32] }:$rs1, ?:{ *:[i32] }:$rs2)
+// CHECK-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/GIMT_Encode2(MyTarget::tst2),
+// CHECK-NEXT: GIR_AddImplicitDef, /*InsnID*/0, GIMT_Encode2(MyTarget::R0), GIMT_Encode2(RegState::Dead),
+// CHECK-NEXT: GIR_AddImplicitDef, /*InsnID*/0, GIMT_Encode2(MyTarget::R1), GIMT_Encode2(RegState::Dead),
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 2,
+
+// CHECK-LABEL: // (sdivrem:{ *:[i32] }:{ *:[i32] } i32:{ *:[i32] }:$rs1, i32:{ *:[i32] }:$rs2)  =>  (tst2:{ *:[i32] }:{ *:[i32] } ?:{ *:[i32] }:$rs1, ?:{ *:[i32] }:$rs2)
+// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::tst2),
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/0, // DstI[rd]
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/2, // rs1
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/3, // rs2
+// CHECK-NEXT: GIR_SetImplicitDefDead, /*InsnID*/0, /*OpIdx for MyTarget::R1*/1,
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // DstI[R0]
+// CHECK-NEXT: GIR_AddRegister, /*InsnID*/1, GIMT_Encode2(MyTarget::R0), /*AddRegisterRegFlags*/GIMT_Encode2(0),
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 3,
+// CHECK-NEXT: GIR_EraseRootFromParent_Done,
+
+// CHECK-LABEL: // (udivrem:{ *:[i32] }:{ *:[i32] } i32:{ *:[i32] }:$rs1, i32:{ *:[i32] }:$rs2)  =>  (tst2:{ *:[i32] }:{ *:[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_RootToRootCopy, /*OpIdx*/2, // rs1
+// CHECK-NEXT: GIR_RootToRootCopy, /*OpIdx*/3, // rs2
+// CHECK-NEXT: GIR_SetImplicitDefDead, /*InsnID*/0, /*OpIdx for MyTarget::R1*/1,
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/1, // DstI[R0]
+// CHECK-NEXT: GIR_AddRegister, /*InsnID*/1, GIMT_Encode2(MyTarget::R0), /*AddRegisterRegFlags*/GIMT_Encode2(0),
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 1,
+// CHECK-NEXT: GIR_EraseRootFromParent_Done,
+
+// CHECK-LABEL: // (get_fpenv:{ *:[i32] })  =>  (tst1:{ *:[i32] })
+// CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::tst1),
+// CHECK-NEXT: GIR_SetImplicitDefDead, /*InsnID*/0, /*OpIdx for MyTarget::R1*/1,
+// CHECK-NEXT: GIR_BuildMI, /*InsnID*/1, /*Opcode*/GIMT_Encode2(TargetOpcode::COPY),
+// CHECK-NEXT: GIR_Copy, /*NewInsnID*/1, /*OldInsnID*/0, /*OpIdx*/0, // DstI[R0]
+// CHECK-NEXT: GIR_AddRegister, /*InsnID*/1, GIMT_Encode2(MyTarget::R0), /*AddRegisterRegFlags*/GIMT_Encode2(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 3f504b73465d21..1f515561a8cd11 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -404,10 +404,11 @@ 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 &Dst, unsigned Start = 0);
+  Expected<action_iterator> importDefRenderers(action_iterator InsertPt,
+                                               RuleMatcher &M,
+                                               BuildMIAction &DstMIBuilder,
+                                               const TreePatternNode &Dst,
+                                               bool IsRoot);
 
   Expected<action_iterator>
   importExplicitUseRenderers(action_iterator InsertPt, RuleMatcher &M,
@@ -420,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.
@@ -1379,8 +1378,9 @@ Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
     CopyToPhysRegMIBuilder.addRenderer<CopyPhysRegRenderer>(PhysInput.first);
   }
 
-  if (auto Error = importExplicitDefRenderers(InsertPt, M, DstMIBuilder, Dst)
-                       .takeError())
+  if (auto Error =
+          importDefRenderers(InsertPt, M, DstMIBuilder, Dst, /*IsRoot=*/true)
+              .takeError())
     return std::move(Error);
 
   if (auto Error = importExplicitUseRenderers(InsertPt, M, DstMIBuilder, Dst)
@@ -1408,8 +1408,8 @@ GlobalISelEmitter::createAndImportSubInstructionRenderer(
   DstMIBuilder.addRenderer<TempRegRenderer>(TempRegID, true);
 
   // Handle additional (ignored) results.
-  InsertPtOrError = importExplicitDefRenderers(std::prev(*InsertPtOrError), M,
-                                               DstMIBuilder, Dst, /*Start=*/1);
+  InsertPtOrError = importDefRenderers(std::prev(*InsertPtOrError), M,
+                                       DstMIBuilder, Dst, /*IsRoot=*/false);
   if (auto Error = InsertPtOrError.takeError())
     return std::move(Error);
 
@@ -1446,13 +1446,13 @@ Expected<action_iterator> GlobalISelEmitter::createInstructionRenderer(
                                        DstI);
 }
 
-Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
+Expected<action_iterator> GlobalISelEmitter::importDefRenderers(
     action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
-    const TreePatternNode &Dst, unsigned Start) {
+    const TreePatternNode &Dst, bool IsRoot) {
   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);
 
@@ -1504,11 +1504,21 @@ Expected<action_iterator> GlobalISelEmitter::importExplicitDefRenderers(
         TempRegID, /*IsDef=*/true, /*SubReg=*/nullptr, /*IsDead=*/true);
   }
 
-  // Implicit defs are not currently supported, mark all of them as dead.
+  // Process implicit defs.
   for (const Record *Reg : DstI->ImplicitDefs) {
     std::string OpName = getMangledRootDefName(Reg->getName());
-    assert(!M.hasOperand(OpName) && "The pattern should've been rejected");
-    DstMIBuilder.setDeadImplicitDef(Reg);
+
+    if (!IsRoot || !M.hasOperand(OpName)) {
+      DstMIBuilder.setDeadImplicitDef(Reg);
+      continue;
+    }
+
+    BuildMIAction &CopyBuilder = M.addAction<BuildMIAction>(
+        M.allocateOutputInsnID(), &Target.getInstruction(RK.getDef("COPY")));
+
+    StringRef PermanentRef = M.getOperandMatcher(OpName).getSymbolicName();
+    CopyBuilder.addRenderer<CopyRenderer>(PermanentRef);
+    CopyBuilder.addRenderer<AddRegisterRenderer>(Target, Reg);
   }
 
   return InsertPt;
@@ -1731,13 +1741,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();
-}
-
 Error GlobalISelEmitter::constrainOperands(action_iterator InsertPt,
                                            RuleMatcher &M, unsigned InsnID,
                                            const TreePatternNode &Dst) {
@@ -2114,13 +2117,14 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
   unsigned DstExpDefs = DstI.Operands.NumDefs,
            DstNumDefs = DstI.ImplicitDefs.size() + DstExpDefs,
            SrcNumDefs = Src.getExtTypes().size();
+
+  bool FoundNoUsePred = false;
   if (DstNumDefs < SrcNumDefs) {
     if (DstNumDefs != 0)
       return failedImport("Src pattern result has more defs than dst MI (" +
                           to_string(SrcNumDefs) + " def(s) vs " +
                           to_string(DstNumDefs) + " def(s))");
 
-    bool FoundNoUsePred = false;
     for (const auto &Pred : InsnMatcher.predicates()) {
       if ((FoundNoUsePred = isa<NoUsePredicateMatcher>(Pred.get())))
         break;
@@ -2133,15 +2137,24 @@ 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 N = std::min(DstExpDefs, SrcNumDefs);
-  for (unsigned I = 0; I < N; ++I) {
-    const auto &DstIOperand = DstI.Operands[I];
+  for (unsigned I = 0; I < SrcNumDefs; ++I) {
+    if (FoundNoUsePred)
+      continue;
 
     OperandMatcher &OM = InsnMatcher.getOperand(I);
+
+    if (I >= DstExpDefs) {
+      const Record *Reg = DstI.ImplicitDefs[I - DstExpDefs];
+      OM.setSymbolicName(getMangledRootDefName(Reg->getName()));
+      M.defineOperand(OM.getSymbolicName(), OM);
+      continue;
+    }
+
     // The operand names declared in the DstI instruction are unrelated to
     // 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.
+    const CGIOperandList::OperandInfo &DstIOperand = DstI.Operands[I];
     OM.setSymbolicName(getMangledRootDefName(DstIOperand.Name));
     M.defineOperand(OM.getSymbolicName(), OM);
 
@@ -2159,11 +2172,6 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
     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



More information about the llvm-commits mailing list