[PATCH] D74688: AMDGPU/GlobalISel: Support llvm.trap and llvm.debugtrap intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:2244-2247
+void AMDGPULegalizerInfo::insertLiveInCopy(MachineIRBuilder &B,
+                                           MachineRegisterInfo &MRI,
+                                           Register LiveIn,
+                                           Register Reg) const {
----------------
hsmhsm wrote:
> arsenm wrote:
> > I think it would be better to have this only take the physical register input, and return the livein virtreg. This wouldn't have the    
> > // Destination virtual register is already defined, just insert copy
> > 
> > case. The intrinsic lowering cases would then be responsible for inserting the extra copy to the expected result
> taken care
This is halfway there. You're still passing in both the live in physical register and the corresponding virtual register. I was thinking you would call getLiveInRegister with just the physical register, and it would be responsible for finding out the virtual register, and inserting the entry block copy by calling insertLiveInCopy. loadInputValue then wouldn't need to worry about ensuring the entry block copy was inserted like it does now.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:3563
+    Register SGPR01(AMDGPU::SGPR0_SGPR1);
+    Register LiveIn = getLiveInRegister(MRI, SGPR01, MRI.getType(SGPR01));
+    if (!loadInputValue(LiveIn, B, Arg))
----------------
Physical registers don't have types. This will just assert. Here you know the type is just LLT::pointer(CONSTANT_ADDRESSS, 64)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74688