[Openmp-commits] [PATCH] D113833: [OpenMP][WIP] Refactor global handling in plugins

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added a comment.

In D113833#3129445 <https://reviews.llvm.org/D113833#3129445>, @JonChesterfield wrote:

> Discussed a bit offline. I'd like to keep the capability to read/write to symbols in the device images, without first having to load the image onto the device and then call into cuda/hsa/etc to access the values that were originally in host memory. readGlobalFromImage exists in this patch, writeGlobalToImage would be very similar in implementation.

We kept the capability to read symbols from device images with this patch. writeGlobalToImage is something that wasn't easily changeable to a generic scheme so I added a TODO and left it. Overall extra cost: 1x H2D.



================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:1231
   {
     // the device_State array is either large value in bss or a void* that
     // needs to be assigned to a pointer to an array of size device_state_bytes
----------------
JonChesterfield wrote:
> The memory optimisation that allocated omptarget_nvptx_device_State one per process instead of once per device image is obsolete with the new runtime
I know. Once we don't need to keep the old for AMD we delete this :)


================
Comment at: openmp/libomptarget/plugins/common/GlobalHandler/GlobalHandler.h:91
+
+/// Helper class to do the heavy lifting when it comes to moving globals between
+/// host and device. Through the DeviceInterfaceTy we access memcpyDtoH and
----------------
JonChesterfield wrote:
> Confusing naming here - coping globals / copying from global symbols?
Not sure what you mean. Suggestions for rewording welcome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113833