llvm / llvm-project

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

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

Open utsumi-fj opened 1 year ago

utsumi-fj commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

@llvm/issue-subscribers-backend-aarch64

john-brawn-arm commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

utsumi-fj commented 1 year ago

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

KiritakeKumi commented 6 months ago

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)