[PATCH] D28538: [CodeGen] [CUDA] Add the ability set default attrs on functions in linked modules.

[Justin Lebar via Phabricator via llvm-commits]

jlebar marked an inline comment as done.
jlebar added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.h:1030
+  /// This is useful for adding attrs to bitcode modules that you want to link
+  /// with but don't control, such as CUDA's libdevice.  When linking with such
+  /// a bitcode library, you probably want to set e.g. its functions'
----------------
hfinkel wrote:
> I think there is an important point here that is missing: for libdevice, we happen to know that this is safe. I think that needs to be in the comment somehow. In general, this is dangerous. libdevice, as I understand it, is specifically designed to make this work (via NVVMReflect).
> 
I've added a comment, but I'm not sure it's quite as dangerous as you seem to think, so maybe my comment isn't scary enough.  I'm happy to continue iterating.

Looking through the specific attrs affected here, for everything other than the fast-math attrs, it seems that we're merely adding attrs to make the code more conservative, but never removing them.

> libdevice, as I understand it, is specifically designed to make this work (via NVVMReflect).

FWIW I wouldn't really say that...  NVVMReflect is an over-general solution to the same problem solved by the denormal-fp-math attr.  Ultimately I would like to nix all of the nvptx-specific FTZ attrs and just use denormal-fp-math.


https://reviews.llvm.org/D28538