From 8d66a39ec6599401e73f573be353e3775bbf94ef Mon Sep 17 00:00:00 2001 From: James R Larrowe Date: Thu, 17 Oct 2024 01:50:27 -0400 Subject: [PATCH 1/3] 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 c53f705a38db8..29c64ba95ff85 100644 --- a/llvm/utils/TableGen/GlobalISelEmitter.cpp +++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp @@ -2023,7 +2023,10 @@ Expected 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 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 94efe98017bf9396926169e190a6d32994adff1f Mon Sep 17 00:00:00 2001 From: James R Larrowe Date: Thu, 17 Oct 2024 02:35:10 -0400 Subject: [PATCH 2/3] 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 0000000000000..d1fb61db8c92b --- /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))]>; From 28094f7ba99af69049221276e10859d561631990 Mon Sep 17 00:00:00 2001 From: James R Larrowe Date: Thu, 17 Oct 2024 04:44:17 -0400 Subject: [PATCH 3/3] switch from CHECK-NOT to implicit-check-not="Skipped pattern" more future-proof --- llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td index d1fb61db8c92b..79af1a336f289 100644 --- a/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td +++ b/llvm/test/TableGen/GlobalISelEmitter-implicit-defs.td @@ -1,9 +1,8 @@ -// 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 +// 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" 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))]>;