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

[Cameron McInally via llvm-dev]

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=