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

[Artem Belevich via Phabricator via cfe-commits]

tra accepted this revision.
tra added subscribers: jyknight, bkramer.
tra added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/CodeGen/CodeGenModule.cpp:1059
+        FD->hasAttr<CUDAGlobalAttr>())
+      MangledName = MangledName + ".stub";
+
----------------
yaxunl wrote:
> 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.
I stand corrected. @jyknight and @bkramer pointed out that appending `.WHATEVER` is currently used for cloning functions and should be OK to do. 


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

https://reviews.llvm.org/D58518