[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Matt Arsenault via Phabricator via llvm-commits]

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