[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[Cameron McInally via Phabricator via cfe-commits]

cameron.mcinally added a comment.

In https://reviews.llvm.org/D53157#1304347, @uweigand wrote:

> OK, let me try to expand on my point 3 above, which appears to have confused everybody :-)


Ah, that makes more sense.

The problem I was missing is when a FENV_ACCESS=OFF operation, like a FDIV, is hoisted into a FENV_ACCESS=ON region. I see it now, but still think that forcing FENV_ACCESS=OFF operations into constrained intrinsics is a big hammer. If there is a way to add barriers around function calls in a FENV_ACCESS=ON region, that would be better.

Stepping way back, the *best* solution is to have the FPEnv implementation shut down unsafe optimizations on an individual basis. Perhaps we should be tagging functions that contain FENV_ACCESS=ON regions. That way unsafe optimization passes, e.g. hoisting, can query the tag to see if they should skip these functions.

> But given that there is still infrastructure missing in the IR optimizers, I also think that at least in the first implementation, we probably should go with the original approach and just use constrained intrinsics everywhere in the function, and possibly add some function attribute that prevent any cross-inlining of functions built with constrained intrinsics with functions built with regular floating-point operations.

Yes, I'll stop moaning about this. This is perfectly suitable for a first implementation.


https://reviews.llvm.org/D53157