[PATCH] D63335: [HIP] Change kernel stub name again

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Is there particular reason you need to switch to this naming scheme?

One issue with this patch is that demanglers will no longer be able to deal with the name. While they do know to ignore .stub suffix, they can't deal with `__device_stub_` prefix.
E.g:

  % c++filt __device_stub___Z10kernelfuncIiEvv
  __device_stub___Z10kernelfuncIiEvv
  % c++filt _Z10kernelfuncIiEvv.stub
  void kernelfunc<int>() [clone .stub]





================
Comment at: clang/lib/CodeGen/CGCUDANV.cpp:222-226
+  assert((CGF.CGM.getContext().getAuxTargetInfo() &&
+          (CGF.CGM.getContext().getAuxTargetInfo()->getCXXABI() !=
+           CGF.CGM.getContext().getTargetInfo().getCXXABI())) ||
+         getDeviceStubName(getDeviceSideName(CGF.CurFuncDecl)) ==
+             CGF.CurFn->getName());
----------------
I'm not sure I understand what exactly this assertion checks.
The condition appears to be true is host/device ABIs are different OR the name of the current function is the same as the (possibly mangled) device-side name + __device_stub_ prefix.

While the first part makes sense, I'm not sure I understand the name comparison part.
Could you tell me more and, maybe, add a comment explaining what's going on here.


================
Comment at: clang/lib/CodeGen/CGCUDANV.cpp:789
+    return Name;
+  return std::move(("__device_stub__" + Name).str());
+}
----------------
I suspect `return "__device_stub__" + Name;` would do. StringRef will convert to std::string and copy elision should avoid unnecessary copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63335