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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to