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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel accepted this revision.
hfinkel added inline comments.
This revision is now accepted and ready to land.


================
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'
----------------
jlebar wrote:
> 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.
This gets the point across. I'm happy.


https://reviews.llvm.org/D28538