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

Tom Stellard via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 21 15:34:29 PST 2016


tstellarAMD added inline comments.


================
Comment at: lib/Target/AMDGPU/AMDGPUISelLowering.h:216
+
+  CCAssignFn *CCAssignFnForCall(CallingConv::ID CC, bool IsVarArg) const;
 };
----------------
ab wrote:
> FWIW I'd keep this in CallLowering (maybe hardcoded?), as you're not already using CCAssignFn in ISelLowering anyway.
I moved the function to the CallLowering class, but I let the implementation in the AMDGPUISelLowering file, because CC_AMDGPU is defined in AMDGPUGenCallingConv.inc


================
Comment at: lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:53
+  setAction({G_STORE, S32}, Legal);
+  setAction({G_STORE, 1, S64}, Legal);
+  setAction({G_STORE, 1, P1}, Legal);
----------------
arsenm wrote:
> ab wrote:
> > arsenm wrote:
> > > ab wrote:
> > > > Huh, why are non-pointer types legal?
> > > What do you mean?
> > Why is it legal to have an 's64' pointer (#1) operand to G_STORE/G_LOAD/G_GEP?
> Oh, I didn't realize integers and pointers were distinct here. 32-bit and 64-bit pointers should both be legal
RegBankSelect will only create copies with scalar types, so if the pointer operands need to be copied to a new register bank, the new value will have a scalar type and not a pointer type.


https://reviews.llvm.org/D26730





More information about the llvm-commits mailing list