arsenm marked an inline comment as done. arsenm added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:2059-2061 + F.removeFnAttrs(AttrsToRemove); + addDenormalModeAttrs(Merged, MergedF32, FuncAttrs); + F.addFnAttrs(FuncAttrs); ---------------- arsenm wrote: > tra wrote: > > IIUIC, this changes denorm mode attributes on the functions with dynamic > > denorm mode that we link in. > > Will that be a problem if the same function, when linked into different > > modules, would end up with different attributes? E.g. if a function is > > externally visible and is intended to be common'ed across multiple modules. > > Should dynamic denorm mode be restricted to the functions private to the > > module only? We do typically internalize linked bitcode for CUDA, but I > > don't think it's something we can always implicitly assume. > > > The whole point of -mlink-builtin-bitcode is to apply the attributes for the > current compilation to what's linked in. The linked functions are always > internalized. The only case where we might not want to internalize is for > weak symbols (but it looks like we do internalize those today, but this is > something I've thought about changing). I'll add a test with a weak library > function > > In the weak case the right thing to do is probably to not change from > dynamic, simply because this linking process is outside of the user's control. > > It turns out we apply attributes prior to internalization. As a separate patch, we can either: # Skip functions that start as interposable, which has an observable change in the IR as it is # Move the link and internalize before setting attributes. This would be unobservable but would catch it if the internalization behavior ever changed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142907/new/ https://reviews.llvm.org/D142907 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
