[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#3986980 <https://reviews.llvm.org/D139785#3986980>, @RalfJung wrote:

>> Comparisons don't look at the sign of NaN. They are not bitwise operations like fabs/fneg.
>
> Yeah, and hence `-NaN > 0.0` is `false`, so that's what I said, no?
>
> My confusion is with this comment
>
>   // Note: This requires nnan to preserve signbit semantics even though the
>   //       signbit of a NAN is insignificant.
>
> since "signbit of a NAN is insignificant" is not correct in general.
>
> I would be less confused by something like
>
>   // Note: This requires nnan to preserve signbit semantics since fcmp ignores
>   //       the signbit of a NAN.

The confusion comes from the IEEE spec itself - the signbit of NAN is never meaningful in real code, but the spec requires that the neg/abs/copysign operations treat the signbit of NAN deterministically anyway. This is what we went back and forth on in the bug report. Ie, we could implement the less strict interpretation, and it should not affect functionality of any real program. The code/test comments were intending to suggest that alternative interpretation if we ever decided that this way was overkill. I'm ok changing the comment if that's preferred.


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

https://reviews.llvm.org/D139785