[PATCH] D26730: AMDGPU/GlobalISel: Add support for simple shaders

[Tom Stellard via Phabricator via llvm-commits]

tstellarAMD added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUCallLowering.cpp:88
+  // FIXME: We need to handle sign/zero extend
+  MIRBuilder.buildLoad(DstReg, PtrReg, *MMO);
+}
----------------
qcolombet wrote:
> I would add a boolean and return false when we encounter such case. At least an assert seems in order :).
We switched to a new ABI since I started working on this, so I don't think we need to handle this case any more, so I'll drop the comment.


================
Comment at: lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:87-93
+  BuildMI(*BB, &I, DL, TII.get(AMDGPU::S_ADD_U32), DstLo)
+          .addOperand(getSubOperand64(I.getOperand(1), AMDGPU::sub0))
+          .addOperand(getSubOperand64(I.getOperand(2), AMDGPU::sub0));
+
+  BuildMI(*BB, &I, DL, TII.get(AMDGPU::S_ADDC_U32), DstHi)
+          .addOperand(getSubOperand64(I.getOperand(1), AMDGPU::sub1))
+          .addOperand(getSubOperand64(I.getOperand(2), AMDGPU::sub1));
----------------
arsenm wrote:
> Can't the register bank be checked here directly to switch to v_add?
Yes, we can but we AMDGPURegisterBankInfo doesn't handle GEP/ADD with VGPRs yet, so the instruction selector will only ever see GEP/ADD with SGPRs.


================
Comment at: lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:222
+  // PseudoSourceValue like GOT.
+  if (!Ptr || isa<UndefValue>(Ptr) || isa<Argument>(Ptr) ||
+      isa<Constant>(Ptr) || isa<GlobalValue>(Ptr))
----------------
arsenm wrote:
> I'm not sure if Ptr == null is valid
It can be null if there is a PseudoSourceValue and you can construct an MMO with a nullptr value too, which I think we used to do.


https://reviews.llvm.org/D26730