[Openmp-commits] [PATCH] D137607: Add interop support for OpenMP AMD GPU plugin

[Jisheng Zhao via Phabricator via Openmp-commits]

jz10 updated this revision to Diff 476739.
jz10 added a comment.

Thanks Johannes

1. "Remove the old plugin API."

The "__tgt_rtl_init_device_info" and "__tgt_rtl_init_async_info" were from CUDA plugin, so I'm not sure if we should remove "__tgt_rtl_init_device_info" since it was not used ourside.

2. "Always allocate an interop object in one place, not in 3, default initialize it."

fixed

3. "If something goes wrong, set an error message and return the object."

fixed, but the checking of API symbols from dynamic lib still returns omp_interop_none,

4. "Pass the interop to the plugin to be initialized."

Made it, but here's one issue, the "vendor_id" and "backend_type_id" used to be const type, but we made them as non-constant now.

5. "Here you write *AsyncInfo but only below you allocate memory for it. Either there is an object and you overwrite it below or there is not and this will segfault."

fixed

6. openmp/libomptarget/src/interop.cpp ln;209 "No error message here?"

This checks if the dynamic lib was loaded successfully or not, if not, then no interop capacity is available, thus omp_interop_none should be enough

7. openmp/libomptarget/src/interop.cpp ln;211 "This API is now unused from the outside, right? Can we get rid of it?"

this is same as 1st issue above


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

https://reviews.llvm.org/D137607

Files:
  openmp/libomptarget/include/interop.h
  openmp/libomptarget/include/omptarget.h
  openmp/libomptarget/include/omptargetplugin.h
  openmp/libomptarget/include/rtl.h
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins/cuda/src/rtl.cpp
  openmp/libomptarget/src/interop.cpp
  openmp/libomptarget/src/rtl.cpp
  openmp/libomptarget/test/api/omp_interop_amdgpu.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D137607.476739.patch
Type: text/x-patch
Size: 13258 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20221120/57b72682/attachment-0001.bin>