[PATCH] D74527: AMDGPU/GlobalISel: Support llvm.trap intrinsic

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Should also handle debug trap, but that may be a separate patch



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1114-1131
+  } else {
+    // Pass queue pointer to trap handler as input
+    // Reference: https://llvm.org/docs/AMDGPUUsage.html#trap-handler-abi
+    unsigned RegNo =
+        MF->getInfo<SIMachineFunctionInfo>()->getQueuePtrUserSGPR();
+    assert(RegNo != AMDGPU::NoRegister);
+    Register SrcReg = Register(RegNo);
----------------
I would factor all of this into a helper function. This is also basically identical to AMDGPULegalizerInfo::loadInputValue, so you could just move this handling into the legalizer


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1117
+    // Reference: https://llvm.org/docs/AMDGPUUsage.html#trap-handler-abi
+    unsigned RegNo =
+        MF->getInfo<SIMachineFunctionInfo>()->getQueuePtrUserSGPR();
----------------
s/unsigned/Regisster


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-trap.ll:1
+; hsa: trap handler enabled
+; RUN: llc -global-isel -mtriple=amdgcn-amd-amdhsa -verify-machineinstrs < %s | FileCheck -check-prefix=LABLE-HSA-TRAP -check-prefix=HSA-TRAP %s
----------------
This isn't an irtranslator, so it shouldn't have it in the name. I would also expect it to be possible to share the test with the DAG tests


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-trap.ll:39-40
+  unreachable
+  store volatile i32 2, i32 addrspace(1)* %arg0
+  ret void
+}
----------------
Remove the dead code


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-trap.ll:42
+}
+
+attributes #0 = { nounwind noreturn }
----------------
Should also have a test where this is used inside a non-entry block, and in a case that uses the stack.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74527/new/

https://reviews.llvm.org/D74527