[Openmp-commits] [PATCH] D125252: [Libomptarget] Add basic support for dynamic shared memory on AMDGPU

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

Looks about as expected, modulo Johannes' comment above. Could we do the getenv call once, instead of per-launch? Somewhere in the massive class constructor perhaps.



================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:1186
+    if (const char *EnvStr = getenv("LIBOMPTARGET_SHARED_MEMORY_SIZE"))
+       packet->group_segment_size = std::stoi(EnvStr);
+    else
----------------
jdoerfert wrote:
> + KernelInfoEntry.group_segment_size;
Yep, need the sum. Possibly worth checking the stoi result is positive and < 64k or so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125252