[llvm] Count implicit defs as well as explicit ones in the GlobalISel TableGen emitter. (PR #112673)

via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 00:53:14 PDT 2024


https://github.com/JL2210 created https://github.com/llvm/llvm-project/pull/112673

`NumDefs` only counts the number of registers in `(outs)`, not any implicit defs specified with `Defs = [...]`

Might want to use `P.getDstRegs.size()` instead of `DstI.ImplicitDefs.size()`, although then I think this would need to be an assert?

>From 3ef675c66218b0b3e45c3bfb285d2704f4c89e39 Mon Sep 17 00:00:00 2001
From: James R Larrowe <larrowe.semaj11 at gmail.com>
Date: Thu, 17 Oct 2024 01:50:27 -0400
Subject: [PATCH 1/2] Count implicit defs as well as explicit ones in the
 GlobalISel TableGen emitter.

NumDefs only counts the number of registers in (outs), not any
implicit defs specified with Defs = [...]
---
 llvm/utils/TableGen/GlobalISelEmitter.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index c53f705a38db8f..29c64ba95ff856 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -2023,7 +2023,10 @@ Expected<RuleMatcher> GlobalISelEmitter::runOnPattern(const PatternToMatch &P) {
   auto &DstI = Target.getInstruction(DstOp);
   StringRef DstIName = DstI.TheDef->getName();
 
-  unsigned DstNumDefs = DstI.Operands.NumDefs,
+  // 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,
            SrcNumDefs = Src.getExtTypes().size();
   if (DstNumDefs < SrcNumDefs) {
     if (DstNumDefs != 0)
@@ -2045,7 +2048,7 @@ 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(DstNumDefs, SrcNumDefs);
+  unsigned N = std::min(DstExpDefs, SrcNumDefs);
   for (unsigned I = 0; I < N; ++I) {
     const TypeSetByHwMode &VTy = Src.getExtType(I);
 

>From 2d9ff1da8c2afb4f688eec8efa4632808d0a03aa Mon Sep 17 00:00:00 2001
From: James R Larrowe <larrowe.semaj11 at gmail.com>
Date: Thu, 17 Oct 2024 02:35:10 -0400
Subject: [PATCH 2/2] Add lit test for implicit defs of physical registers

---
 .../TableGen/GlobalISelEmitter-implicit-defs.td     | 13 +++++++++++++
 1 file changed, 13 insertions(+)
 create mode 100644 llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td

diff --git a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
new file mode 100644
index 00000000000000..d1fb61db8c92b4
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td
@@ -0,0 +1,13 @@
+// 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
+
+include "llvm/Target/Target.td"
+include "GlobalISelEmitterCommon.td"
+
+// CHECK-NOT: 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: 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
+let Uses = [B0] in
+def tst2 : I<(outs), (ins), [(set B0, (add B0, 1))]>;



More information about the llvm-commits mailing list