[PATCH] D119027: [AMDGPU][NFC]: Emit metadata for hidden_heap_v1 kernarg

[Changpeng Fang via Phabricator via llvm-commits]

cfang added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributes.def:22
 AMDGPU_ATTRIBUTE(HOSTCALL_PTR, "amdgpu-no-hostcall-ptr")
+AMDGPU_ATTRIBUTE(HEAP_PTR, "amdgpu-no-heap-ptr")
 AMDGPU_ATTRIBUTE(WORKGROUP_ID_X, "amdgpu-no-workgroup-id-x")
----------------
sameerds wrote:
> It seems we need to document these attributes in AMDGPUUsage, which I missed for no-hostcall-ptr. 
> 
> https://llvm.org/docs/AMDGPUUsage.html#llvm-ir-attributes
Can you suggest the description for both no-hostcall and no-heap-ptr? Thanks.  


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:521
     // initial call to the implicitarg_ptr intrinsic.
-    auto DoesNotLeadToHostcallPtr = [&](Instruction &I) {
+    auto DoesNotLeadToMemoryLoc = [&](Instruction &I) {
       auto &Call = cast<CallBase>(I);
----------------
sameerds wrote:
> b-sumner wrote:
> > We could potentially be more specific with KernargLoc instead of MemoryLoc.
> Or more importantly, the outer function needs a more specific name ... it is checking whether the supplied offset is being accessed from the implicitarg_ptr base.
Will update as suggested. Thanks.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:521
     // initial call to the implicitarg_ptr intrinsic.
-    auto DoesNotLeadToHostcallPtr = [&](Instruction &I) {
+    auto DoesNotLeadToMemoryLoc = [&](Instruction &I) {
       auto &Call = cast<CallBase>(I);
----------------
cfang wrote:
> sameerds wrote:
> > b-sumner wrote:
> > > We could potentially be more specific with KernargLoc instead of MemoryLoc.
> > Or more importantly, the outer function needs a more specific name ... it is checking whether the supplied offset is being accessed from the implicitarg_ptr base.
> Will update as suggested. Thanks.
Can we use "funcRetrievesImplicitKernarg(...) ?


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

https://reviews.llvm.org/D119027