Skip to content

isless macro throws FE_INVALID when NAN is specified to the argument #59924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
utsumi-fj opened this issue Jan 11, 2023 · 13 comments
Open

isless macro throws FE_INVALID when NAN is specified to the argument #59924

utsumi-fj opened this issue Jan 11, 2023 · 13 comments

Comments

@utsumi-fj
Copy link

utsumi-fj commented Jan 11, 2023

The isless macro throws FE_INVALID when NAN is specified to the argument.
When one of the arguments of isless is unordered, isless should not throw "invalid" floating-point exception.

Reproducer:

test.c

#include <stdio.h>
#include <math.h>
#include <fenv.h>

int main() {
  long double nan128 = (long double)NAN;
  if (isless(nan128, 1.0))
    puts("test");

  if (fetestexcept(FE_INVALID))
    puts("FE_INVALID");

  return 0;
}

Compilation commands and execution result:

$ clang --version
clang version 15.0.6 (https://github.com/llvm/llvm-project.git 088f33605d8a61ff519c580a71b1dd57d16a03f8)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /work03/lang/llvm/llvm/15.0.6/bin
$ clang test.c -lm
$ ./a.out
FE_INVALID
$ clang test.c -lm -O1
$ ./a.out
$

This issue was detected in AArch64 machine.

@rotateright
Copy link
Contributor

The program as written invokes undefined behavior, so there is no bug.
From the C standard section 7.6.1:
"If part of a program tests floating-point status flags, sets floating-point control modes, or runs under non-default mode settings, but was translated with the state for the FENV_ACCESS pragma ‘‘off’’, the behavior is undefined."

You must use "#pragma STDC FENV_ACCESS ON" to avoid that. See discussion in #8472.

@utsumi-fj
Copy link
Author

@rotateright Thanks for your answer. I understood that this is not a bug for now.
After adding #pragma STDC FENV_ACCESS ON, the warning '#pragma FENV_ACCESS' is not supported on this target was emitted as follows.

$ cat fenv_access_on.c
#include <stdio.h>
#include <math.h>
#include <fenv.h>

int main() {
#pragma STDC FENV_ACCESS ON

  long double nan128 = (long double)NAN;
  if (isless(nan128, 1.0))
    puts("test");

  if (fetestexcept(FE_INVALID))
    puts("FE_INVALID");

  return 0;
}

$ clang fenv_access_on.c -lm
fenv_access_on.c:6:14: warning: '#pragma FENV_ACCESS' is not supported on this target - ignored [-Wignored-pragmas]
#pragma STDC FENV_ACCESS ON
             ^
1 warning generated.
$ ./a.out
FE_INVALID

When #pragma FENV_ACCESS is supported, I think that this behavior needs to be fixed.

@utsumi-fj
Copy link
Author

@rotateright I tried newer version of the compiler as follows, and the warning '#pragma FENV_ACCESS' is not supported on this target was not emitted.
Maybe https://reviews.llvm.org/D138143 permits to specify #pragma FENV_ACCESS for AArch64.

$ clang --version
clang version 16.0.0 (https://github.com/llvm/llvm-project.git 2d6e280223cca4e44882245feb06e0c50d4c6375)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /work03/lang/llvm/llvm/main/bin
$ cat fenv_access_on.c
#include <stdio.h>
#include <math.h>
#include <fenv.h>

int main() {
#pragma STDC FENV_ACCESS ON

  long double nan128 = (long double)NAN;
  if (isless(nan128, 1.0))
    puts("test");

  if (fetestexcept(FE_INVALID))
    puts("FE_INVALID");

  return 0;
}
$ clang fenv_access_on.c -lm
$ ./a.out
FE_INVALID

FE_INVALID was thrown in the above program with #pragma STDC FENV_ACCESS ON.
So is this a bug?

@rotateright
Copy link
Contributor

Yes, it seems like a bug if the compiler supports FENV_ACCESS for your target now, but it shows an exception.
I do not see the problem running on x86. What happens if you use "double" rather than "long double"?
If there is a way to show the execution bug on godbolt that would be helpful:
https://godbolt.org/z/3f397ocn5

@utsumi-fj
Copy link
Author

@rotateright Thanks for replying.

What happens if you use "double" rather than "long double"?

If the type is "double", FE_INVALID was not thrown in my program and target(AArch64).
It seems that goldbolt can execute programs only on X86 machine.

In AArch64, if the type is "double", instructions that raise the floating point exception is not generated since the software floating point library is not used e.g. https://godbolt.org/z/14Tqhfr87.
In this case, the instruction that compares (double)NAN and 1.0 is fcmp d0 d1, which does not raise a floating point exception.

But, In AArch64, if the type is "long double", __lttf2, which is a software floating point library function, is called with arguments (double)NAN and 1.0 directly e.g. https://godbolt.org/z/x3Kj4b7dx.
In my environment, __lttf2 includes the instructions that raise the floating point exception.

So, I think that there need to be the instructions checking that both of the arguments passed to __lttf2 are not NAN.

@rotateright
Copy link
Contributor

rotateright commented Jan 13, 2023

Thanks for the analysis @utsumi-fj !
It sounds like a lowering problem of the constrained FP intrinsic, so I'm going to tag this as an AArch64 codegen problem for now (although the bug/solution may be target-independent).

You can show LLVM IR on godbolt by adding "-emit-llvm" to the compiler flags. I'll paste the example here for easier access:

target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64-unknown-linux-gnu"

@.str = private unnamed_addr constant [5 x i8] c"test\00", align 1
@.str.1 = private unnamed_addr constant [11 x i8] c"FE_INVALID\00", align 1

define dso_local i32 @main() #0 {
  %1 = alloca i32, align 4
  %2 = alloca fp128, align 16
  store i32 0, ptr %1, align 4
  %3 = call fp128 @llvm.experimental.constrained.fpext.f128.f32(float 0x7FF8000000000000, metadata !"fpexcept.strict") #4
  store fp128 %3, ptr %2, align 16
  %4 = load fp128, ptr %2, align 16
  %5 = call fp128 @llvm.experimental.constrained.fpext.f128.f64(double 1.000000e+00, metadata !"fpexcept.strict") #4
  %6 = call i1 @llvm.experimental.constrained.fcmp.f128(fp128 %4, fp128 %5, metadata !"olt", metadata !"fpexcept.strict") #4
  br i1 %6, label %7, label %9

7:                                                ; preds = %0
  %8 = call i32 @puts(ptr noundef @.str) #4
  br label %9

9:                                                ; preds = %7, %0
  %10 = call i32 @fetestexcept(i32 noundef 1) #5
  %11 = icmp ne i32 %10, 0
  br i1 %11, label %12, label %14

12:                                               ; preds = %9
  %13 = call i32 @puts(ptr noundef @.str.1) #4
  br label %14

14:                                               ; preds = %12, %9
  ret i32 0
}

declare fp128 @llvm.experimental.constrained.fpext.f128.f32(float, metadata) #1

declare fp128 @llvm.experimental.constrained.fpext.f128.f64(double, metadata) #1

declare i1 @llvm.experimental.constrained.fcmp.f128(fp128, fp128, metadata, metadata) #1

declare i32 @puts(ptr noundef) #2

declare i32 @fetestexcept(i32 noundef) #3

attributes #0 = { noinline nounwind optnone strictfp uwtable "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "strictfp" "target-cpu"="generic" "target-features"="+fp-armv8,+neon,+outline-atomics,+v8a,-fmv" }
attributes #1 = { nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite) }
attributes #2 = { "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+fp-armv8,+neon,+outline-atomics,+v8a,-fmv" }
attributes #3 = { nounwind "frame-pointer"="non-leaf" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="generic" "target-features"="+fp-armv8,+neon,+outline-atomics,+v8a,-fmv" }
attributes #4 = { strictfp }
attributes #5 = { nounwind strictfp }

!llvm.module.flags = !{!0, !1, !2, !3, !4, !5}
!llvm.ident = !{!6}

!0 = !{i32 7, !"Dwarf Version", i32 4}
!1 = !{i32 1, !"wchar_size", i32 4}
!2 = !{i32 8, !"PIC Level", i32 2}
!3 = !{i32 7, !"PIE Level", i32 2}
!4 = !{i32 7, !"uwtable", i32 2}
!5 = !{i32 7, !"frame-pointer", i32 1}
!6 = !{!"clang version 16.0.0 (https://github.com/llvm/llvm-project.git 8efb8f776abf1cb66107c42b47e6a127828c4db0)"}

@llvmbot
Copy link
Member

llvmbot commented Jan 13, 2023

@llvm/issue-subscribers-backend-aarch64

@john-brawn-arm
Copy link
Collaborator

This looks like it could be a bug in the implementation of __lttf2 that you're using. https://gcc.gnu.org/onlinedocs/gccint/Soft-float-library-routines.html doesn't say anything explicit about floating-point exceptions, but neither the libgcc nor compiler-rt implementations will raise an exception when an input is NaN.

@utsumi-fj
Copy link
Author

@john-brawn-arm Thanks for your comment.
In libgcc or glibc, __lttf2 seems to be implemented to raise a floating point exception when either of the inputs is NAN.

__lttf2 seems to be the alias of __letf2.
__letf2 is defined in https://github.com/gcc-mirror/gcc/blob/master/libgcc/soft-fp/letf2.c.
The macro FP_CMP_Q used by __letf2 is defined in https://github.com/gcc-mirror/gcc/blob/master/libgcc/soft-fp/quad.h#L184.
The macro _FP_CMP used by FP_CMP_Q is defined in https://github.com/gcc-mirror/gcc/blob/master/libgcc/soft-fp/op-common.h#L1303.
If either of the inputs of __letf2 is NAN, _FP_CMP_CHECK_NAN used by _FP_CMP raises a floating point exception.
In https://github.com/gcc-mirror/gcc/blob/master/libgcc/soft-fp/op-common.h#L1254, the exception seems to be raised.

Also, I tried compiler-rt. When using compiler-rt, this issue was not detected i.e. a floating point exception was not raised.
I think that it is because exception handling code does not exist in compiler-rt's implementation.
I cannot find exception handling code in compiler-rt's __leXf2__.

According to the document https://gcc.gnu.org/onlinedocs/gccint/Soft-float-library-routines.html, the semantics of __letf2 is described as follows.

These functions return a value less than or equal to zero if neither argument is NaN, and a is less than or equal to b.

The above is not saying about when either of the arguments is NaN. And the following sentence is also written in this document.

Do not rely on this implementation; only the semantics documented below are guaranteed.

So, I think that NAN should not be passed to __lttf2 and so on directly.

@kawashima-fj
Copy link
Member

Probably this is an issue mentioned in D71897 and is marked as FIXME in the SelectionDAG source.

@utsumi-fj
Copy link
Author

@kawashima-fj Thanks for your comment. I didn't know the discussion in D71897. I agree with you.

@KiritakeKumi
Copy link

Everyone, I also encountered a similar problem on the riscv64 platform, and the reason may be the same as this problem.

Code:

import numpy as np

code = 'g' # long double
fnan = np.array(np.nan, dtype=code)
fone = np.array(1.0, dtype=code)

with np.errstate(all='raise'):
    print(np.floor_divide(fnan, fone))

Error message:
FloatingPointError: invalid value encountered in floor divide

Clang version:
Debian clang version 16.0.6(21)

@ngoldbaum
Copy link

There's a PR to add a special case to NumPy on aarch64 for clang to work around this: numpy/numpy#28232

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants