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

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added inline comments.


================
Comment at: openmp/libomptarget/plugins/amdgpu/impl/system.cpp:137
 std::vector<std::map<std::string, atl_symbol_info_t>> SymbolInfoTable;
 
 bool g_atmi_initialized = false;
----------------
these ^ are probably not the ideal structures. We convert char* into std::string at each call site, and map() is expected to be slower than unordered_map for the ~random access lookup.

vector<unordered_map<const char*, info_t>> with the custom equality / hasher for C strings is a reasonable choice.


================
Comment at: openmp/libomptarget/plugins/amdgpu/impl/system.cpp:1042
+
+    // Kernel name can be derived from symbol by removing the .kd suffix
+    const std::string Suffix = ".kd";
----------------
JonChesterfield wrote:
> Can we use the foo.kd form everywhere instead of truncating it?
^that turns out to be worse, truncating is the way to go. Probably cleaner to mutate the name[] buffer than do extra things with strings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102691