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

[Ulrich Weigand via llvm-commits]

uweigand added a comment.

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

> In https://reviews.llvm.org/D22685#494797, @uweigand wrote:
>
> > 1.) Does this actually have to be platform-specific?  I mean, is there any platform where a SNaN constant *can* be stored in converted form?  I would have expected that the type-conversion operation needed to load a "shrunk" SNaN constant would be required to actually trigger the "signaling" part of the SNaN (i.e. raise an invalid operation exception) on all platforms ...
>
>
> This was my initial thought too. It doesn't seem to be a problem on x86 (we get the right value printed and no exceptions), which suggests that it's being handled somehow. I'm not sure about other platforms however.
>  I left this fix as platform-specific to reduce the possible impact, but if there are no expected drawbacks from disabling it globally, then I'm willing to do so.


Hmm, I've had a bit of a closer look into x86 here.  First of all, it seems we're only "shrinking" constants in the x86 extended format.  And for this format, there's special code in APFloat::convert that marks any conversion of an x86 extended format SNaN to double as "inexact", and therefore the generic shrinking code doesn't do it (because isValueValidForType returns false).

The current code just has a somewhat cryptic comment:

  // x86 has some unusual NaNs which cannot be represented in any other
  // format; note them here.

but the original code as checked in here:
http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20081006/068140.html
explicitly says:

  // We mark this as Inexact if we are losing information.  This happens
  // if are shifting out something other than 0s, or if the x87 long
  // double input did not have its integer bit set (pseudo-NaN), or if the
  // x87 long double input did not have its QNan bit set (because the x87
  // hardware sets this bit when converting a lower-precision NaN to
  // x87 long double).

The remark in parenthesis seems to match exactly the problem on System z.

I actually haven't seen any case where LLVM would in fact generate loads of a "shrunk" SNaN constant.  Do you have any such case?

B.t.w. the original patch had a test case which seems to have gotten lost in this iteration :-)


https://reviews.llvm.org/D22685