[X86][GlobalISel] Use GIntrinsic during custom intrinsic selection

[e-kud]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-x86

Author: Evgenii Kudriashov (e-kud)

Changes

---

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

2 Files Affected:

• (modified) llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp (+1-1)
• (added) llvm/test/CodeGen/X86/GlobalISel/select-custom-intrinsic.mir (+27)

diff --git a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
index 26932ba2c8e242..34d0daf4296990 100644
--- a/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
+++ b/llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
@@ -1843,7 +1843,7 @@ bool X86InstructionSelector::selectIntrinsicWSideEffects(
   assert(I.getOpcode() == TargetOpcode::G_INTRINSIC_W_SIDE_EFFECTS &&
          "unexpected instruction");
 
-  if (I.getOperand(0).getIntrinsicID() != Intrinsic::trap)
+  if (cast<GIntrinsic>(I).getIntrinsicID() != Intrinsic::trap)
     return false;
 
   BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(X86::TRAP));
diff --git a/llvm/test/CodeGen/X86/GlobalISel/select-custom-intrinsic.mir b/llvm/test/CodeGen/X86/GlobalISel/select-custom-intrinsic.mir
new file mode 100644
index 00000000000000..85c59b1566164b
--- /dev/null
+++ b/llvm/test/CodeGen/X86/GlobalISel/select-custom-intrinsic.mir
@@ -0,0 +1,27 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 4
+# RUN: llc -mtriple=x86_64-linux-gnu -run-pass=instruction-select -global-isel-abort=2 -pass-remarks-missed='gisel*' 2>&1 %s -o - | FileCheck %s --check-prefixes=CHECK
+# RUN: llc -mtriple=i386-linux-gnu -run-pass=instruction-select -global-isel-abort=2 -pass-remarks-missed='gisel*' 2>&1 %s -o - | FileCheck %s --check-prefixes=CHECK
+
+# CHECK: remark: <unknown>:0:0: cannot select: %2:gpr(s8) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.x86.atomic.sub.cc), %0:gpr(p0), %1:gpr(s32), 4 (in function: intrinsic_returns)
+
+...
+---
+name:            intrinsic_returns
+legalized:       true
+regBankSelected: true
+body:             |
+  bb.1:
+
+    ; CHECK-LABEL: name: intrinsic_returns
+    ; CHECK: [[DEF:%[0-9]+]]:gpr(p0) = IMPLICIT_DEF
+    ; CHECK-NEXT: [[C:%[0-9]+]]:gpr(s32) = G_CONSTANT i32 1
+    ; CHECK-NEXT: [[INT:%[0-9]+]]:gpr(s8) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.x86.atomic.sub.cc), [[DEF]](p0), [[C]](s32), 4
+    ; CHECK-NEXT: $al = COPY [[INT]](s8)
+    ; CHECK-NEXT: RET 0, implicit $al
+    %0:gpr(p0) = IMPLICIT_DEF
+    %1:gpr(s32) = G_CONSTANT i32 1
+    %2:gpr(s8) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.x86.atomic.sub.cc), %0(p0), %1(s32), 4
+    $al = COPY %2(s8)
+    RET 0, implicit $al
+
+...

[arsenm]

generic intrinsics should not use G_INTRINSIC, this shouldn't have been reachable

[e-kud]

That's interesting. It seems that llvm.trap is a special case as well as llvm.debugtrap and llvm.ubsantrap. Comparing to other intrinsics we can't lower it into combination of GlobalISel opcodes. So it is alive until instruction selection and then we custom select it.

If we take a look at SelectionDAG it lowers the intrinsic into ISD::TRAP. It seems that we can create G_TRAP/G_DEBUGTRAP/G_UBSANTRAP opcodes and lower intrinsics during irtranslator. I see that only 3 targets have a custom lowering for traps: AArch64, X86 and RISCV. So it shouldn't be a problem to migrate them all at once to proper lowering. What do you think, does it worth it?

[arsenm]

Yes, that's how I would expect this to work

[arsenm]

This is fine until the new G_* instructions are added

[e-kud]

@tschuett @arsenm thank you for the review! The PR with new intrinsic opcodes has been merged instead: #84941