Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GISel] Add RegState::Define to temporary defs in apply patterns #77425

Merged
merged 3 commits into from Jan 9, 2024

Conversation

s-barannikov
Copy link
Contributor

Previously, registers created for temporary defs in apply patterns were rendered as uses, resulting in machine verifier errors.

Previously, registers created for temporary defs in apply patterns were
rendered as uses, resulting in machine verifier errors.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 9, 2024

@llvm/pr-subscribers-llvm-globalisel

Author: Sergei Barannikov (s-barannikov)

Changes

Previously, registers created for temporary defs in apply patterns were rendered as uses, resulting in machine verifier errors.


Full diff: https://github.com/llvm/llvm-project/pull/77425.diff

2 Files Affected:

  • (added) llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-temp-defs.td (+56)
  • (modified) llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp (+1-1)
diff --git a/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-temp-defs.td b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-temp-defs.td
new file mode 100644
index 00000000000000..4e473355e14c36
--- /dev/null
+++ b/llvm/test/TableGen/GlobalISelCombinerEmitter/match-table-temp-defs.td
@@ -0,0 +1,56 @@
+// RUN: llvm-tblgen -I %p/../../../include -gen-global-isel-combiner \
+// RUN:     -combiners=MyCombiner %s | FileCheck %s
+
+// Checks that temporary registers defined in apply patterns
+// are emitted with RegState::Define.
+
+include "llvm/Target/Target.td"
+include "llvm/Target/GlobalISel/Combine.td"
+
+def MyTargetISA : InstrInfo;
+def MyTarget : Target { let InstructionSet = MyTargetISA; }
+
+def Test0 : GICombineRule<
+  (defs root:$dst),
+  (match (G_ADD $dst, $lhs, $rhs)),
+  (apply (G_UDIVREM $tmp, $dst, $lhs, $rhs))
+>;
+
+def Test1 : GICombineRule<
+  (defs root:$dst),
+  (match (G_ADD $dst, $lhs, $rhs)),
+  (apply (G_UDIVREM $dst, $tmp, $lhs, $rhs))
+>;
+
+def Test2 : GICombineRule<
+  (defs root:$dst),
+  (match (G_ADD $dst, $lhs, $rhs)),
+  (apply (G_ADD $tmp, 0, $lhs),
+         (G_ADD $dst, $tmp, $rhs))
+>;
+
+def MyCombiner: GICombiner<"GenMyCombiner", [
+  Test0,
+  Test1,
+  Test2,
+]>;
+
+// CHECK:       // Combiner Rule #0: Test0
+// CHECK-NEXT:  GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_UDIVREM),
+// CHECK-NEXT:  GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
+// CHECK-NEXT:  GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
+// CHECK-NEXT:  GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // lhs
+// CHECK-NEXT:  GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/2, // rhs
+
+// CHECK:       // Combiner Rule #1: Test1
+// CHECK-NEXT:  GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_UDIVREM),
+// CHECK-NEXT:  GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
+// CHECK-NEXT:  GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
+// CHECK-NEXT:  GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // lhs
+// CHECK-NEXT:  GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/2, // rhs
+
+// CHECK:       // Combiner Rule #2: Test2
+// CHECK-NEXT:  GIR_BuildMI, /*InsnID*/0, /*Opcode*/GIMT_Encode2(TargetOpcode::G_ADD),
+// CHECK-NEXT:  GIR_AddTempRegister, /*InsnID*/0, /*TempRegID*/0, /*TempRegFlags*/GIMT_Encode2(RegState::Define),
+// CHECK-NEXT:  GIR_AddSimpleTempRegister, /*InsnID*/0, /*TempRegID*/1,
+// CHECK-NEXT:  GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/1, // lhs
diff --git a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
index 89aca87a28ec0d..d95f323d9d6b46 100644
--- a/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelCombinerEmitter.cpp
@@ -2316,7 +2316,7 @@ bool CombineRuleBuilder::emitInstructionApplyPattern(
           M.actions_begin(), getLLTCodeGenOrTempType(Ty, M), TempRegID);
     }
 
-    DstMI.addRenderer<TempRegRenderer>(TempRegID);
+    DstMI.addRenderer<TempRegRenderer>(TempRegID, /*IsDef=*/true);
   }
 
   // Render MIFlags

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this got this far

@s-barannikov s-barannikov merged commit 06286a5 into llvm:main Jan 9, 2024
4 checks passed
@s-barannikov s-barannikov deleted the gisel/temp-defs-register-flags branch January 9, 2024 14:55
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…m#77425)

Previously, registers created for temporary defs in apply patterns were
rendered as uses, resulting in machine verifier errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants