llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.9k stars 11.51k forks source link

@llvm.maxnum.* does not handle signaling NaNs the same way as IEEE 754-2008 #27737

Open 1ba3d143-a64b-4671-82b2-0b31cfb91709 opened 8 years ago

1ba3d143-a64b-4671-82b2-0b31cfb91709 commented 8 years ago
Bugzilla Link 27363
Version trunk
OS All
CC @asb,@frasercrmck,@sunfishcode,@lenary

Extended Description

The @​llvm.maxnum.* intrinsic is documented like this:

Follows the IEEE-754 semantics for maxNum, which also match for libm’s fmax.

If either operand is a NaN, returns the other non-NaN operand. Returns NaN only if both operands are NaN.

If one operand is a signaling NaN, this behavior doesn't match IEEE 754-2008:

@llvm.maxnum.f64(1.0, qNaN) -> 1.0
@llvm.maxnum.f64(1.0, sNaN) -> 1.0

IEEE 754-2008:

maxNum(1.0, qNaN) -> 1.0
maxNum(1.0, sNaN) -> qNaN

I wonder if this changed in the 2008 revision?

The Aarch64 backend translates an fmax() library call to an fmaxnm instruction which has the 754-2008 semantics for signaling NaNs.

frasercrmck commented 3 years ago

This bug is quite old now but I just came across it myself in testing. My question is that now that the intrinsic itself is better documented for signalling NaNs, are we still considering this an AArch64 backend bug?

Unlike the IEEE-754 2008 behavior, this does not distinguish between signaling and quiet NaN inputs. If a target’s implementation follows the standard and returns a quiet NaN if either input is a signaling NaN, the intrinsic lowering is responsible for quieting the inputs to correctly return the non-NaN input (e.g. by using the equivalent of llvm.canonicalize).

If I write an AArch64 program which uses these intrinsics e.g.

%call1 = tail call float @​llvm.minnum.f32(float 0x42D303C360000000, float 0x7FF4F5F5C0000000)

Then %call1 is incorrectly given a quiet NaN value (0x7fa7afae).

The X86 and RISCV backends (the only two I've tried) both get this right and return the non-NaN input.

asb commented 6 years ago

I hadn't spotted that change vs the 2.2 and earlier specs, thanks. It should be noted it's not yet in a published version of the spec, but at this point it's very likely to be included in the 2.3 user-level ISA manual.

sunfishcode commented 6 years ago

According to this:

https://github.com/riscv/riscv-isa-manual/commit/cd20cee7efd9bac7c5aa127ec3b451749d2b3cce

It looks like RISC-V has been changed to avoid the 754-2008 behavior.

asb commented 6 years ago

Adding myself to the CC list. FWIW, this issue also affects RISC-V, which has fmin.s, fmin.d, fmax.s and fmax.d instructions with semantics matching IEEE 754-2008.

arsenm commented 1 year ago

The quoted text contradicts itself. The current LangRef clarifies these are really fmin/fmax, and should really be renamed as such. Ideally we would have both fmin/fmax and minnum/maxnum