[PATCH] D57416: [AMDGPU] Support emitting GOT relocations for function calls

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIISelLowering.cpp:3488-3492
+    if (DefI->getOpcode() == AMDGPU::S_LOAD_DWORDX2_IMM) {
+      AddrReg = DefI->getOperand(1).getReg();
+      DefI = MRI.getVRegDef(AddrReg);
+    }
+    assert(DefI->getOpcode() == AMDGPU::SI_PC_ADD_REL_OFFSET);
----------------
scott.linder wrote:
> arsenm wrote:
> > Looking for a specific load opcode looks concerning. Where is this coming from? I would think we need a pseudo at this point
> SITargetLowering::LowerGlobalAddress adds the load after creating the SI_PC_ADD_REL_OFFSET pseudo. Would it be better to add a new pseudo for just the load, or create a pseudo to include the existing PC relative calculation and the load, i.e. SI_LOAD_PC_ADD_REL_OFFSET? I don't think the load needs to be bundled to enforce an ordering or anything.
I'm worried about selecting a different opcode, and the possibility of some transform somewhere obscuring the address.

I think I looked at this before, and other targets use different call opcodes based on the address type. I think I did it this way because unlike other targets, we don't embed the call address in the actual call instruction. I think using the second pseudo would be closer to this, though I still don't like relying on getVRegDef here.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57416