[PATCH] D58518: [HIP] change kernel stub name

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: lib/CodeGen/CodeGenModule.cpp:1059
+        FD->hasAttr<CUDAGlobalAttr>())
+      MangledName = MangledName + ".stub";
+
----------------
tra wrote:
> Changing mangled name exposes this change to a wider scope of potential issues. Is the mangled name still valid after this change? I.e. will external demanglers have problem with it? Is `.` a valid symbol in mangled names on all platforms we support?
> 
> I think changing the name here is way too late and we should figure out how to change the stub name when we generate it.
> 
> @echristo Eric, what do you think?
The external demangler can still demangle this name. e.g. c++filt will demangle this name and add [clone .stub] after that.

As far as I can see this function is only called in codegen to map FunctionDecl names to LLVM function names. I've tried this change with real ML frameworks and it works.

Changing at this place is not too late. The stub function name is requested at multiple places in codegen, not just at the emitting of stub function definition. For template kernel function, the emitting of stub function definition is deferred after emitting of the call of the stub function. Basically, codegen needs to find the corresponding LLVM stub function by getMangledName first, then by GetOrCreateLLVMFunction. If we do not change getMangledName, codegen will not get the correct stub function name consistently at all places. That's why the previous patch does not work.


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

https://reviews.llvm.org/D58518