[PATCH] D120265: AMDGPU: Use the implicit kernargs for code object version 5

Sameer Sahasrabuddhe via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 16 21:34:20 PDT 2022


sameerds added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:546
+    AAPointerInfo::OffsetAndSize OAS(
+        AMDGPU::ImplicitArg::HEAP_PTR_OFFSET_COV5, 8);
     return funcRetrievesImplicitKernelArg(A, OAS);
----------------
I think we should keep using the original getHeapPtrImplicitArgPosition(). Hardcoding the enum here doesn't necessarily make the code more readable. And later if we have a different value in COV6, we will end up reintroducing a check for the code-object-version anyway. That check can be encapsulated within the get...ArgPosition() family of functions.


================
Comment at: llvm/lib/Target/AMDGPU/SIDefines.h:786
+enum Offset : unsigned {
+  HOSTCALL_PTR_OFFSET_PRIOR_COV5 = 24,
+  HOSTCALL_PTR_OFFSET_COV5 = 80,
----------------
This should be "UPTO_COV4". Or if we really want to say COV5, then "BEFORE_COV5" or "PRE_COV5". But to me, "UPTO_COV4" is the clearest.


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

https://reviews.llvm.org/D120265



More information about the llvm-commits mailing list