[Openmp-commits] [PATCH] D102691: [AMDGPU][Libomptarget] Remove global KernelNameMap

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

Couple of style comments above. I think this works as-is, so we have confirmation that the map of strings can go. Which is great!



================
Comment at: openmp/libomptarget/plugins/amdgpu/impl/system.cpp:880
+
     atl_kernel_info_t info = {0, 0, 0, 0, 0, 0, 0, 0, 0, {}, {}, {}};
 
----------------
I think the above works but could be clearer as:
(kernelName + ".kd" ) != symbolName

kernelName can drop out of scope after this test (probably move it into a nested brace to make that clear), so we could mutate it instead of making a new string to probably save an allocation. But in general, '(foo + ".kd") == bar' seems simpler than the find calls.


================
Comment at: openmp/libomptarget/plugins/amdgpu/impl/system.cpp:932
     // create a map from symbol to name
     DEBUG_PRINT("Kernel symbol %s; Name: %s; Size: %lu\n", symbolName.c_str(),
                 kernelName.c_str(), kernel_segment_size);
----------------
Lets drop the debug print and comment here now that the map is gone


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102691