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

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a comment.

I like the direction. Could we hold it for a day or so? I'd like to check through the uses of the kernel name to see if there's a missing edge case, or if we can simplify this a step further.

It looks like the msgpack data always contains the foo and the foo.kd strings, under different keys. I wonder if that's something we can rely on the compiler emitting.



================
Comment at: openmp/libomptarget/plugins/amdgpu/impl/system.cpp:874
     msgpack_errors += map_lookup_string(element, ".symbol", &symbolName);
     if (msgpack_errors != 0) {
       printf("[%s:%d] %s failed\n", __FILE__, __LINE__,
----------------
I think this is the earliest point we can check the foo to foo.kd mapping is in place, so that we can error out if it isn't


================
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";
----------------
Can we use the foo.kd form everywhere instead of truncating it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102691