[llvm-dev] PR43374 - when should comparing NaN values raise a floating point exception?

[Sanjay Patel via llvm-dev]

Let's change the example to eliminate suspects:
  #include <math.h>
  int is_nan(float x) {
    /*
      The following subclauses provide macros that are quiet (non
floating-point exception raising)
      versions of the relational operators, and other comparison macros
that facilitate writing
      efficient code that accounts for NaNs without suffering the
‘‘invalid’’ floating-point exception.
    */
    return isunordered(x, x);
  }

The comment text is from 7.12.14 of the C standard draft. I'm hoping to
avoid any scenario under which it is ok to raise an exception in that code
(eliminate any questions about the clang front-end behavior / FENV_ACCESS).

As IR from clang with no optimization, this becomes a bunch of load/store
with:
  %cmp = fcmp uno double %conv, %conv1

Ok, so far? "fcmp uno" - http://llvm.org/docs/LangRef.html#fcmp-instruction
:
uno: yields true if either operand is a QNAN.

EarlyCSE/InstCombine reduce that fcmp to:
  %cmp = fcmp uno float %x, 0.000000e+00

Still good? Same fcmp predicate, but we replaced a repeated use of "%x"
with a zero constant to aid optimization.

Now, send the optimized IR to codegen:
define i32 @is_nan(float %x) {
  %cmp = fcmp uno float %x, 0.000000e+00
  %r = zext i1 %cmp to i32
  ret i32 %r
}

$ llc -o - fpexception.ll -mtriple=armv7a
  vmov s0, r0
  mov r0, #0
  vcmpe.f32 s0, s0
  vmrs APSR_nzcv, fpscr
  movwvs r0, #1
  bx lr

We produced "vcmpe" for code that should never cause an FP exception. ARM
codegen bug?

On Tue, Oct 1, 2019 at 5:45 AM Kristof Beyls <Kristof.Beyls at arm.com> wrote:

> Hi,
>
> I’ve been investigating https://bugs.llvm.org/show_bug.cgi?id=43374,
> which is about clang/llvm producing code that triggers a floating point
> exception when x is NaN, when targeting ARM, in the below code example.
>
> int bar(float x) {
>   return x!=x ? 0 : 1;
> }
>
> The C99 standard states in section 7.12.14:
>
> """
> The relational and equality operators support the usual mathematical
> relationships between numeric values. For any ordered pair of numeric
> values exactly one of the relationships — less, greater, and equal — is
> true. Relational operators may raise the ‘‘invalid’’ floating-point
> exception when argument values are NaNs.
> """
>
> My interpretation of that paragraph is that it's OK for <, <=, > and >= to
> raise an exception when argument values are NaNs. It is not OK for == an !=
> to raise an exception when argument values are NaNs.
>
> Therefore,
>
> int bar(float x) {
>   return x!=x ? 0 : 1;
> }
>
> should not produce an exception when x is NaN, and hence a vcmp rather
> than vcmpe instruction should be produced when generating ARM code for
> this.
>
> http://llvm.org/viewvc/llvm-project?rev=294945&view=rev introduced
> support for generating vcmp instead of vcmpe for equality comparisons.
> How come vcmpe is generated for (x!=x)?
>
> The answer is that InstCombine transforms the equality comparison into an
> "ordered comparison”. Before InstCombine:
> define dso_local i32 @bar(float %x) local_unnamed_addr {
> entry:
>   %cmp = fcmp une float %x, %x
>   %cond = select i1 %cmp, i32 0, i32 1
>   ret i32 %cond
> }
>
> After InstCombine:
> define dso_local i32 @bar(float %x) local_unnamed_addr #0 {
> entry:
>   %cmp = fcmp ord float %x, 0.000000e+00
>   %cond = zext i1 %cmp to i32
>   ret i32 %cond
> }
>
> Please note that on other backends like x86 or AArch64, this InstCombine
> doesn’t trigger floating point exception behaviour since those backends
> don’t seem to be producing any instructions for fcmp that raise floating
> point exceptions on NaNs.
>
> My question here is: how to fix this behaviour? Or: which part in the
> compilation flow is wrong?
> Reading through various standards and specifications, I’m getting confused
> to what the best fix would be:
>
>
>    - https://llvm.org/docs/LangRef.html#floating-point-environment states
>    "The default LLVM floating-point environment assumes that
>    floating-point instructions do not have side effects. Results assume the
>    round-to-nearest rounding mode. No floating-point exception state is
>    maintained in this environment. Therefore, there is no attempt to create or
>    preserve invalid operation (SNaN) or division-by-zero exceptions.”
>    This suggests that if we want to retain floating point exception
>    behaviour in the compilation flow, we shouldn’t be using the “default LLVM
>    floating-point environment”, but rather something else. Presumably the
>    constrained intrinsics? However, when I look at the constrained intrinsics
>    definition, it seems (
>    http://llvm.org/docs/LangRef.html#constrained-floating-point-intrinsics)
>    there is no constrained intrinsic for the floating point comparison
>    operation. Should there be one?
>    - If the default floating-point environment assumes that
>    floating-point instructions do not have side effects, why does the Arm
>    backend lower floating point comparison to vcmpe rather than vcmp? The
>    revision history suggests this has been this way since the initial creation
>    of the ARM backend. Should this behaviour be changed and vcmp be produced
>    rather than vcmpe? And only later, once the generation of constrained
>    floating point intrinsics is implemented should backends start producing
>    signalling floating point comparisons for floating point comparison
>    constrained intrinsics (assuming they’ll exist by then)?
>    - Or alternatively, there is a good reason to keep on producing vcmpe
>    as is today, and instcombine just shouldn’t convert “fcmp une” into “fcmp
>    ord”?
>    - Or as yet another alternative, instcombine is just fine converting
>    “fcmp une” into “fcmp ord”, and it’s the ARM backend that should produce
>    vcmp rather than vcmpe also for “unordered” comparisons, next to equality
>    comparisons?
>
>
> Thanks,
>
> Kristof
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191001/37031e40/attachment-0001.html>