llvm dev - Oct 2019 - PR43374 - when should comparing NaN values raise a floating point exception?

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/21f1291c/attachment-0001.html>

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>

On Tue, Oct 1, 2019 at 5:45 AM Kristof Beyls via llvm-dev
<llvm-dev at lists.llvm.org> 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?
Yes, there should be a number of constrained FP compares. Work had
started on this, but was sidelined. I believe that Kevin took over
this work. Kevin?
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=DwIGaQ&c=slrrB7dE8n7gBJbeO0g-IQ&r=O_4M49EtSpZ_-BQYeigzGv0P4__noMcSu2RYEjS1vKs&m=Qo0Q_p6SYp6xQeS0FnNcTNsL49ruqS1IRJVlRLKxLFg&s=NRnIoHZkkjeMuMLb7Yy6-_V4Nin8zRTSXu9wVQW6LJA&e=

Ulrich offered to do it instead since I expect he can get it done much faster
than me. Instead I'm doing SIToFP and UIToFP. Ulrich said he wasn't
going to be able to get to it for a couple of weeks, but that was a week or two
ago.

Ulrich?

-----Original Message-----
From: Cameron McInally <cameron.mcinally at nyu.edu> 
Sent: Tuesday, October 01, 2019 10:21 AM
To: Kristof Beyls <Kristof.Beyls at arm.com>
Cc: llvm-dev <llvm-dev at lists.llvm.org>; nd <nd at arm.com>; Kevin
Neal <Kevin.Neal at sas.com>
Subject: Re: [llvm-dev] PR43374 - when should comparing NaN values raise a
floating point exception?

EXTERNAL

On Tue, Oct 1, 2019 at 5:45 AM Kristof Beyls via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> Hi,
>
> I’ve been investigating
https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.llvm.org%2Fshow_bug.cgi%3Fid%3D43374&amp;data=02%7C01%7CKevin.Neal%40sas.com%7C1c2441d90c5b4d3c4e6408d7467aa4cc%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C637055364875886682&amp;sdata=7LPxf1%2BSB9lxwSjQhmEW%2FgxI997UVSo6h1XkGUrnapo%3D&amp;reserved=0,
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.
>
>
https://nam02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fviewvc%2Fllvm-project%3Frev%3D294945%26view%3Drev&amp;data=02%7C01%7CKevin.Neal%40sas.com%7C1c2441d90c5b4d3c4e6408d7467aa4cc%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C637055364875886682&amp;sdata=BeuNr6A9lgTnILxtPIAyGIonkL9SfeFn1sRQ9hb1ayg%3D&amp;reserved=0
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://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm.org%2Fdocs%2FLangRef.html%23floating-point-environment&amp;data=02%7C01%7CKevin.Neal%40sas.com%7C1c2441d90c5b4d3c4e6408d7467aa4cc%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C637055364875886682&amp;sdata=g5QujHhKvsyWLOZggSu45jIbW%2Bmf7BXForHwvo6%2FMoI%3D&amp;reserved=0
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
(https://nam02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fllvm.org%2Fdocs%2FLangRef.html%23constrained-floating-point-intrinsics&amp;data=02%7C01%7CKevin.Neal%40sas.com%7C1c2441d90c5b4d3c4e6408d7467aa4cc%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C637055364875886682&amp;sdata=NiDp01OiszjRk1wvQpAAdCMZsY99Q5AYWqK3QQ1hUiE%3D&amp;reserved=0)
there is no constrained intrinsic for the floating point comparison operation.
Should there be one?
Yes, there should be a number of constrained FP compares. Work had
started on this, but was sidelined. I believe that Kevin took over
this work. Kevin?
> 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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
>
https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Furldefense.proofpoint.com%2Fv2%2Furl%3Fu%3Dhttps-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev%26d%3DDwIGaQ%26c%3DslrrB7dE8n7gBJbeO0g-IQ%26r%3DO_4M49EtSpZ_-BQYeigzGv0P4__noMcSu2RYEjS1vKs%26m%3DQo0Q_p6SYp6xQeS0FnNcTNsL49ruqS1IRJVlRLKxLFg%26s%3DNRnIoHZkkjeMuMLb7Yy6-_V4Nin8zRTSXu9wVQW6LJA%26e&amp;data=02%7C01%7CKevin.Neal%40sas.com%7C1c2441d90c5b4d3c4e6408d7467aa4cc%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C637055364875886682&amp;sdata=9iAk3T6gfAadX8ze8qGk2kmN%2BIbJXRLoXXFK4qIemMI%3D&amp;reserved=0=

Thank you very much Sanjay for this analysis.

I indeed also think that vcmpe should not be produced for regular fcmp LLVM-IR
instructions.
Maybe it should be produced for constrained FP compare intrinsics, when those
get introduced.

I’ve put up a patch for review at https://reviews.llvm.org/D68463 to no longer
produce vcmpe but instead vcmp in the Arm backend.

Thanks!

Kristof

On 1 Oct 2019, at 15:44, Sanjay Patel <spatel at
rotateright.com<mailto:spatel at rotateright.com>> wrote:

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<mailto: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/20191004/f0a2d249/attachment.html>

* Sanjay Patel via llvm-dev <llvm-dev at lists.llvm.org> [2019-10-01
09:44:54 -0400]:
> 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).
isunordered (like isnan) is a different operation from x!=x,
in particular it does not signal even on signaling nans
while x!=x does.
> 
> 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.
why is that ok?

here you need an unordered-not-equal, but got an unordered-compare,
the former is a silent ieee754 operation the latter is signaling.

as noted earlier this is wrong for signaling nans, because any fcmp
would signal for them, but the lack of snan support can be forgiven,
however here even qnans fail to remain silent, that's a bug.
> 
> 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.
no, this is different from even x!=x not to mention isunordered(x,x).

clang does not support FENV_ACCESS but repeatedly claimed in relevant
bug reports that users who care should use -O0, but it seems to create
broken compares even at -O0, so there is no way to get c99 conforming
behaviour.
> 
> 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?
sorry, the arm code gen is right here, the bug is in clang.
> 
> 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
> >
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev