[PATCH] D139785: [InstCombine] preserve signbit semantics of NAN with fold to fabs

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D139785#3987002 <https://reviews.llvm.org/D139785#3987002>, @RalfJung wrote:

> The signbit is never meaningful in code that only treats float values as approximation to mathematical real numbers, but LLVM must work on other code, too. For that reason I think the mindset of "it is not meaningful but ..." is confusing, and easily leads to misunderstandings and bugs such as the one this change is fixing.

I'm not convinced that this is really a bug, but I'm ok changing the wording. :)

By default, LLVM IR implements a subset of IEEE FP to allow optimizations that IEEE does not:
https://llvm.org/docs/LangRef.html#floating-point-environment
(also note that we support targets that are even less IEEE-compliant than the IR spec, so we don't do some transforms that would be allowed if we could assume an IEEE-compliant target)

So it's up to us to decide if this quirk of NAN and signbit-op behavior is worth implementing. This will make Alive2 more useful, and I'm hoping it won't harm optimization in real code. If it does harm optimization, we could switch back (leave this transform as-is). In other words, we would make it explicit that the signbit of NAN is meaningless as it was (briefly) implemented in Alive2. But then we would lose some other optimization as noted in the bug report.


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

https://reviews.llvm.org/D139785