[PATCH] D22685: Disable shrinking of SNaN constants on SystemZ

[Ulrich Weigand via llvm-commits]

uweigand added a comment.

In https://reviews.llvm.org/D22685#496061, @uweigand wrote:

> In https://reviews.llvm.org/D22685#496030, @colpell wrote:
>
> > Hmm, that definitely explains why x86 isn't running into this issue.
> >  In that case, is the general approach of not shrinking SNaNs in the first place better, or would it make sense to add more special code to APFloat to avoid "inexact" conversions on SystemZ as well?
> >  My feeling is that more backend-specific in APFloat is a bit messy, though it has the potential to fix other cases where we're converting a SNaN (not that I'm currently aware of any).
>
>
> Thinking about that, I'm not sure whether the APFloat.convert behavior is really correct.  In many cases, it is possible to convert a SNaN in to another SNaN in a different format exactly.  (Not all cases, of course.  Also, the x86 extended format "pseudo-NaNs" are really special, and can never be converted exactly.)


To be more specific, I was trying to say the APFloat.convert behavior for non-x86 architectures seems correct, so we should't be adding any SNaN-specific code there.  (I think the behavior on *x86* may not be fully correct for non-pseudo SNaNs in the extended format, but that's a different story ...)

> But simply never doing constant shrinking of a signaling NaN seems the best way to me right now ...


That still seems the right fix to me: simply never shrink SNaN values on any platform.

Since this is then a cross-platform change, you might want to add some floating-point experts to the approvers list.

In https://reviews.llvm.org/D22685#496087, @colpell wrote:

> Ah, sorry, I should have been more clear -- that code snippet causes the SNaN to be shrunk and converted to a QNaN specifically on SystemZ. I don't have any code that reproduces the issue on other platforms (on x86, the APFloat behaviour is presumably preventing the shrinking from happening).


OK, understood.


https://reviews.llvm.org/D22685