[PATCH] D106362: [FPEnv][InstSimplify] Enable more folds for constrained fadd

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added a comment.

In D106362#3031370 <https://reviews.llvm.org/D106362#3031370>, @kpn wrote:

> In D106362#3031030 <https://reviews.llvm.org/D106362#3031030>, @sepavloff wrote:
>
>> In D106362#3030871 <https://reviews.llvm.org/D106362#3030871>, @kpn wrote:
>>
>>> In D106362#3030856 <https://reviews.llvm.org/D106362#3030856>, @sepavloff wrote:
>>>
>>>> In D106362#3030790 <https://reviews.llvm.org/D106362#3030790>, @kpn wrote:
>>>>
>>>>> Changing the way the existing transforms check for FMF.noNaNs() sounds like a different ticket that needs to be done before this one can progress. Unless I misunderstood?
>>>>
>>>> I know nothing about such change.
>>>
>>> Your proposed changes around SNaN will affect the non-constrained cases.
>>
>> I think the optimization `fadd X, -0 ==> X` in general case (not `FMF.noNaNs()`) is incorrect. The obtained values must be identical to what would produce hardware, otherwise it is not an optimization. If you think that such change deserves a separate patch, no problem. It seems to me that this patch could establish correct folding even if it changes non-constrained operation as well.
>
> I really don't think we should be changing how LLVM handles the ebIgnore cases. LLVM currently treats SNaN and QNaN as the same except where it doesn't, and I believe we should keep that behavior unchanged when ignoring exceptions. At the very least we shouldn't be disabling any currently enabled optimizations when ignoring exceptions. @lebedev.ri or @jcranmer ?

It is actually a bug. None of the operation in IEEE-754 compatible implementation may produce SNaN. If some code that would produce QNaN in runtime produces SNaN due to constant folding, such optimization is invalid. A user might use the expression `x + 0.0` just to convert SNaN to QNaN, it won't work in the case of such optimization.

> Our closest approximation to a strict IEEE-754 mode currently is with ebStrict / "fpexcept.strict" usage, but that isn't documented as being strictly IEEE-754 compliant. If someone wants to go through and add a strict -754 mode at some point then they can. But that's a different project.

Most hardware FP implementations adhere IEEE-754. As LLVM IR is by design target-independent, non-IEEE-754 implementation hardly is interesting. Also, `ebIgnore` is only an optimization hint, it should not change the semantics of the `add` operation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106362/new/

https://reviews.llvm.org/D106362