[PATCH] D91516: [AMDGPU] Support for device scope shared variables

[Mahesha S via Phabricator via llvm-commits]

hsmhsm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUDeviceScopeSharedVariable.cpp:59-60
+//      computed as in above item C.
+//   F. Within each kernel, type cast the corresponding single big shared memory
+//      layout to `char*`, and pass this type-casted pointer and the kernel
+//      number as new function arguments along the call graph path(s) so that
----------------
arsenm wrote:
> No reason to mention C types
Fixed


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUDeviceScopeSharedVariable.cpp:940
+  // One is LDS layout base pointer which is of type `char*`, and other one is
+  // kernel number which is of type `Int64`.
+  //
----------------
arsenm wrote:
> Shouldn't refer to C types. Also is it really necessary to use i64 for the kernel number? 32-bits should be enough?
Fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91516