arsenm added a comment.

In D142907#4288555 <https://reviews.llvm.org/D142907#4288555>, @efriedma wrote:

> If you have a library function that's built with 
> "denormal-fp-math"="dynamic,dynamic", you can link it into code built in any 
> mode, and LTO should be able to propagate that mode from the caller to the 
> callee.  That doesn't require clang to do anything special; you can just 
> specify -fdenormal-fp-math=dynamic while building the library, and the user 
> specifies -fdenormal-fp-math=ieee while building their code.

That's essentially what this does. I think the part you are missing is the 
existing special treatment of the builtin device library functions. The default 
set of attributes for the current translation unit is forcibly set on functions 
in bitcode libraries linked in and internalized with -mlink-builtin-bitcode. We 
need to logically merge with the current translation unit's mode, or else we're 
potentially breaking the linked in function. The main reason I'm doing this in 
the first place is to move towards a model with less special treatment of these 
libraries.

> I guess you're worried specifically about ODR inline functions, defined in 
> headers?  The user specifies a specific mode because they know their code 
> honors it... but the user might not be aware of the effect on functions 
> defined in library headers.  Other libraries in the same binary might use the 
> same header, but specify a different mode.  So if the user specifies a 
> denormal mode, we should ignore it for ODR inline functions, because they 
> didn't actually mean to apply the denormal mode to those definitions?

No, the clang changes are to handle the headerless bitcode-only device 
libraries only. The user is supposed to be unaware the builtin libraries exist. 
They're an implementation detail managed by the clang driver 
(-mlink-builtin-bitcode is a cc1 only flag) and have a special contract with 
the compiler.

> I'm not sure about applying those semantics automatically; I don't think 
> there's any precedent in clang for anything like this.  The closest thing I 
> can think of is -fvisibility-inlines-hidden.  I'd prefer to RFC it separately 
> from the rest of the patch, and loop in clang frontend owners, since the 
> precedent we set here will apply to other sorts of attributes.

This isn't new, isn't end user facing, and isn't general purpose. This is the 
minimum required update to the existing -mlink-builtin-bitcode handling.


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

Reply via email to