Fix #7868

[Keno]

Somewhere in the LLVM 3.4 timeframe somebody decided that the constant folder should always return positive NaNs. Anyway, a couple of months somebody finally realized that you can't implement IEEE negate if your Constant Folder doesn't produce negative NaNs, so the constant folder for fsub was made to preserve sign bits. This is the resulting mess 😢 Also IEEE floating point experts, is there anything in there (other than the negate argument), that would let me argue that -1.0*NaN should be -NaN rather than +NaN?

[Keno]

Also turns out this doesn't actually fix the issue in all cases. sigh

[Keno]

Actually does anybody know what the hardware does here?

[Keno]

Ok, this version should be correct. It was pointed out to me that there is no way to build a compliant fneq out of arithmetic operations and logical ones should be used instead. @JeffBezanson please have a look and merge.

[JeffBezanson]

I am very worried that throwing in a bit cast and integer operation for such a simple floating-point op might mess with the vectorizer or otherwise hurt performance.

IEEE says the sign bit of a NaN is only specified for copy, negate, abs, and copySign.

After the change to the constant folder for fsub, why doesn't it work just to use fsub?

[Keno]

Because I don't actually think that's what the hardware does.

[JeffBezanson]

Do you mean the hardware 0 - x does not implement IEEE negate, and LLVM lacks a negate instruction, so accessing the hardware neg is impossible? If so that is a very serious design problem in LLVM.

[Keno]

That's my impression, yes. I couldn't actually find a reference on what the hardware does, but from my initial tests last night it does not seem to match IEEE negate.

[Keno]

Also, note that I'm referring to the hardware subtraction instruction here. With the proper optimizations, LLVM will turn it into the vxorps, which does behave like IEEE negate.

[JeffBezanson]

That's not surprising; the hardware is correct there. Subtraction is of course a different operation.

Does this code, with integer bitwise ops, also get optimized to vxorps?

[Keno]

No, it does not unfortunately. I was planning to address that in LLVM though.

[StefanKarpinski]

The high-level argument why LLVM's behavior is incorrect here is that this is supposed to be a pure optimization and have no user-visible effect. Producing different bits is highly user visible. It doesn't really matter that the values involved are NaNs. By the IEEE standard, they can for many of these operations return any NaN they want – but it should be the same NaN regardless of whether constant folding is done or not. The fact that the hardware instruction flips the sign bit in the optimized version indicates to me that LLVM really ought to be flipping the sign in both versions.

[Keno]

Well, technically the sign bit is undefined by IEEE if either input or output are NaNs so LLVM thinks it's fine. We all know how much LLVM likes to exploit undefined behavior.

[JeffBezanson]

I think it's fine for an optimization to change a result that's undefined. Without that flexibility many low-level optimizations would be way too difficult or blocked entirely.

[Keno]

Yes, I agree, It's just too bad that there's really no way implement negate other than bitcasting.

[JeffBezanson]

I tried a C test program, and clang emits

  %4 = fsub fast double -0.000000e+00, %3

for negation. Maybe that -0.0 does the trick within llvm somehow?

[Keno]

No, I doubt it. I'm pretty sure with optimizations disabled, LLVM will always emit the hardware subtraction.

[Keno]

Looks like I was wrong. The -0.0 does indeed do the trick on LLVM 3.5.

[Keno]

This looks like it works reliably now.

[JeffBezanson]

Awesome. Thank goodness we can take advantage of hacks they need to make clang work :)

Does the -0.0 trick really not work on LLVM 3.3? Was clang just buggy then?

[Keno]

@JeffBezanson The constant folder definitely doesn't. This was only fixed on Jun 8: llvm-mirror/llvm@bef256f.

[JeffBezanson]

Ok, then this will have to do.

[Keno]

It was pointed out to me that the reason fsub -0.0, x works, but fsub 0.0, x doesn't is that the latter does not negate if x==+0.0