[Openmp-commits] [PATCH] D140973: [OpenMP][AMDGPU] Support of cov5 in the next gen plugin

Johannes Doerfert via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Wed Jan 4 07:53:55 PST 2023

jdoerfert added a comment.

Thanks for updating the new plugin too. Some comments below.

Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp:1666
+    new (AMDKernel)
+        AMDGPUKernelTy(KernelEntry.name, ExecModeGlobal.getValue(), Cov);
Is this a per-kernel property or a per image property? If it's the latter, maybe add a TODO to move it to a image abstraction once we have a AMD specific one.

Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp:2134
 /// handler.
 struct AMDGPUGlobalHandlerTy final : public GenericGlobalHandlerTy {
   /// Get the metadata of a global from the device. The name and size of the
COV is AMD-GPU specific I assume, right? If so, put the implementation in here and a default impl into the generic part. Also, add documentation to each new method, please.

Comment at: openmp/libomptarget/plugins-nextgen/amdgpu/utils/UtilitiesRTL.h:29
+  return Version < ELF::ELFABIVERSION_AMDGPU_HSA_V5 ? 56 : 256;
I think we want to keep the layout, eventually we'll need it, right?

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list