Skip to content

[TableGen][GISel] Remove check for LLT when emitting renderers #121144

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

Merged
merged 2 commits into from
Dec 26, 2024

Conversation

s-barannikov
Copy link
Contributor

@s-barannikov s-barannikov commented Dec 26, 2024

Types used in the destination DAG of a pattern should not matter for GlobalISel. All necessary checks are emitted in the form of matchers when traversing the source DAG.

In particular, the check prevented importing patterns containing iPTR in the middle of the destination DAG.

This reduces the number of skipped patterns on Mips and RISCV:

Mips   1270  -> 1212  (-58)
RISCV 42165 -> 42088  (-77)

Most of these patterns are for atomic operations.

Types used in the destination DAG of a pattern should not matter for
GlobalISel. All necessary checks are emitted in the form of matchers
when traversing the source DAG.

In particular, the check prevented importing patterns containing
iPTR in the middle of the destination DAG.

This reduces the number of skipped patterns on Mips and RISCV:
Mips   1270  -> 1212  (-58)
RISCV 42165 -> 42088  (-77)

Most of these patterns are for atomic operations.
@llvmbot
Copy link
Member

llvmbot commented Dec 26, 2024

@llvm/pr-subscribers-tablegen

@llvm/pr-subscribers-llvm-globalisel

Author: Sergei Barannikov (s-barannikov)

Changes

Types used in the destination DAG of a pattern should not matter for GlobalISel. All necessary checks are emitted in the form of matchers when traversing the source DAG.

In particular, the check prevented importing patterns containing iPTR in the middle of the destination DAG.

This reduces the number of skipped patterns on Mips and RISCV:

Mips   1270  -> 1212  (-58)
RISCV 42165 -> 42088  (-77)

Most of these patterns are for atomic operations.


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

2 Files Affected:

  • (modified) llvm/test/TableGen/GlobalISelEmitter/OverloadedPtr.td (+22-4)
  • (modified) llvm/utils/TableGen/GlobalISelEmitter.cpp (-9)
diff --git a/llvm/test/TableGen/GlobalISelEmitter/OverloadedPtr.td b/llvm/test/TableGen/GlobalISelEmitter/OverloadedPtr.td
index c70211d6652250..8eb43add9381a6 100644
--- a/llvm/test/TableGen/GlobalISelEmitter/OverloadedPtr.td
+++ b/llvm/test/TableGen/GlobalISelEmitter/OverloadedPtr.td
@@ -4,10 +4,32 @@
 include "llvm/Target/Target.td"
 include "GlobalISelEmitterCommon.td"
 
+def GPR : RegisterClass<"MyTarget", [i32, i64], 32, (add R0)>;
+
 let TargetPrefix = "mytarget" in {
     def int_mytarget_anyptr : Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty]>;
 }
 
+// Check that iPTR in the destination DAG doesn't prevent the pattern from being imported.
+
+// CHECK: GIM_RootCheckType, /*Op*/0, /*Type*/GILLT_s32,
+// CHECK-NEXT: GIM_CheckMemorySizeEqualToLLT, /*MI*/0, /*MMO*/0, /*OpIdx*/0,
+// CHECK-NEXT: GIM_CheckAtomicOrdering, /*MI*/0, /*Order*/(uint8_t)AtomicOrdering::NotAtomic,
+// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/0, /*RC*/GIMT_Encode2(MyTarget::GPR32RegClassID),
+// CHECK-NEXT: // MIs[0] src1
+// CHECK-NEXT: GIM_CheckPointerToAny, /*MI*/0, /*Op*/1, /*SizeInBits*/0,
+// CHECK-NEXT: GIM_RootCheckRegBankForClass, /*Op*/1, /*RC*/GIMT_Encode2(MyTarget::GPRRegClassID),
+// CHECK-NEXT: // (ld:{ *:[i32] } GPR:{ *:[iPTR] }:$src1)<<P:Predicate_unindexedload>><<P:Predicate_load>>  =>  (ANYLOAD:{ *:[i32] } GPR:{ *:[iPTR] }:$src1)
+// CHECK-NEXT: GIR_MutateOpcode, /*InsnID*/0, /*RecycleInsnID*/0, /*Opcode*/GIMT_Encode2(MyTarget::ANYLOAD),
+// CHECK-NEXT: GIR_RootConstrainSelectedInstOperands,
+// CHECK-NEXT: // GIR_Coverage, 0,
+// CHECK-NEXT: GIR_Done,
+
+let hasSideEffects = 1 in {
+  def ANYLOAD : I<(outs GPR32:$dst), (ins GPR32:$src1),
+            [(set GPR32:$dst, (load iPTR:$src1))]>;
+}
+
 // Ensure that llvm_anyptr_ty on an intrinsic results in a
 // GIM_CheckPointerToAny rather than a GIM_CheckType.
 //
@@ -20,10 +42,6 @@ let TargetPrefix = "mytarget" in {
 // CHECK-NEXT: GIM_CheckCxxInsnPredicate, /*MI*/0, /*FnId*/GIMT_Encode2(GICXXPred_MI_Predicate_frag_anyptr),
 // CHECK-NEXT: // (intrinsic_w_chain:{ *:[i32] } {{[0-9]+}}:{ *:[iPTR] }, GPR32:{ *:[i32] }:$src)<<P:Predicate_frag_anyptr>>  =>  (ANYLOAD:{ *:[i32] } GPR32:{ *:[i32] }:$src)
 // CHECK-NEXT: GIR_BuildRootMI, /*Opcode*/GIMT_Encode2(MyTarget::ANYLOAD),
-let hasSideEffects = 1 in {
-  def ANYLOAD : I<(outs GPR32:$dst), (ins GPR32:$src1),
-            [(set GPR32:$dst, (load GPR32:$src1))]>;
-}
 
 def frag_anyptr : PatFrag<(ops node:$src),
                    (int_mytarget_anyptr node:$src),
diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp
index 0b910096b0528f..f0fb11625883ea 100644
--- a/llvm/utils/TableGen/GlobalISelEmitter.cpp
+++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp
@@ -1246,15 +1246,6 @@ Error GlobalISelEmitter::importNamedNodeRenderer(
     if (N.getNumResults() != 1)
       return failedImport("node does not have one result " + to_string(N));
 
-    std::optional<LLTCodeGen> OpTyOrNone;
-    ArrayRef<TypeSetByHwMode> ChildTypes = N.getExtTypes();
-    if (ChildTypes.front().isMachineValueType())
-      OpTyOrNone = MVTToLLT(ChildTypes.front().getMachineValueType().SimpleTy);
-
-    // TODO: Remove this check. Types in the destination DAG should not matter.
-    if (!OpTyOrNone)
-      return failedImport("node has unsupported type " + to_string(N));
-
     if (R->isSubClassOf("ComplexPattern")) {
       auto I = ComplexPatternEquivs.find(R);
       if (I == ComplexPatternEquivs.end())

@s-barannikov s-barannikov merged commit 4a92c27 into llvm:main Dec 26, 2024
8 checks passed
@s-barannikov s-barannikov deleted the tablegen/gisel/remove-llt-check branch December 26, 2024 14:45
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 26, 2024

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building llvm at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/10910

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49779.cpp (980 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug53727.cpp (981 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug50022.cpp (982 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/test_libc.cpp (983 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/wtime.c (984 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/std_complex_arithmetic.cpp (985 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/bug49021.cpp (986 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/complex_reduction.cpp (987 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49021.cpp (988 of 993)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (989 of 993)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1235.861225

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.

4 participants