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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
