[Openmp-commits] [PATCH] D102600: [AMDGPU][Libomptarget] Rename & move g_executables to private

Mon May 17 06:55:21 PDT 2021

JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

Ok, looks reasonable. There's a comment on the abo e comment, but could equally just drop the comment. Vector of hsa_executable is probably obvious enough for naming.

================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:331

+  // Kernel information for each device
+  std::vector<hsa_executable_t> HSAExecutables;
----------------
This is per loaded code object, not per device. Kernel information is a bit misleading, it's whatever HSA chose to abstract over the code object.

================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:548
+      Err = hsa_executable_destroy(HSAExecutables[I]);
+      if (Err != HSA_STATUS_SUCCESS) {
+        printf("[%s:%d] %s failed: %s\n", __FILE__, __LINE__,
----------------
This handling of errors isn't ideal, but matches the current behaviour closely enough

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102600