[flang-commits] [flang] [flang][AMDGPU] Convert math ops to AMD GPU library calls instead of libm calls (PR #99517)

[Jan Leyonberg via flang-commits]

jsjodin wrote:

> > @arsenm Overall, is there a problem with using the OCML functions uniformly, even if they're trivial?
> 
> Yes, especially when they're trivial. The cases that just map to an llvm intrinsic add system complexity for no benefit.
> 

Having a more uniform interface reduces complexity. That interface should be the llvm intrinsics (or maybe libm calls), but there are holes and forcing the higher level transforms to know about what intrinsics that are implemented and not in the backend increases complexity in my opinion.

> > That leaves the ability for them to become non-trivial in the future
> 
> Fundamentally we should be treating it more as a runtime library for the compiler and not a user facing abstraction. Right now it's trying to act like a user library or libm and compiler-rt all at the same time when they have different/incompatible constraints. 

If I understand this correctly the main issue is that clang and flang use OCML at all, and it should be purely internal to LLVM.
OCML is only used within the LLVM projects from what I can tell. It doesn't seem to be user-facing, since it is not mentioned in the ROCm documentation anymore (not for a long time), so we can perhaps assume we control where and how it is used. Once a real solution is implemented we should be able to just delete this pass and let the standard lowering passes do the work. In the mean time I think it is reasonable to use OCML as the uniform interface.

What are your opinions on adding the "implements" attribute as long term solution? This would add an attribute to tell the compiler that a function implements an intrinsic something like __attribute__ implements(llvm.pow). I was looking into this, but I don't have time to work on it right now. The discussion thread is here:
https://discourse.llvm.org/t/nvptx-codegen-for-llvm-sin-and-friends/58170/33


https://github.com/llvm/llvm-project/pull/99517