Skip to content

clang/llvm don't support C99 FP rounding mode pragmas (FENV_ACCESS etc) #8472

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
llvmbot opened this issue Sep 7, 2010 · 32 comments
Open
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" floating-point Floating-point math

Comments

@llvmbot
Copy link
Member

llvmbot commented Sep 7, 2010

Bugzilla Link 8100
Version trunk
OS Linux
Reporter LLVM Bugzilla Contributor
CC @andykaylor,@delcypher,@DimitryAndric,@emaste,@ecnelises,@ismail,@kcc,@sunfishcode,@zygoloid,@rnk,@rotateright,@tavianator,@tydeman,@vinc17fr,@vadz,@yabinc,@yuanfang-chen,@zamazan4ik

Extended Description

Hello,

consider this code:

#include <fenv.h>
#include <assert.h>

int main(){
double a=1.1;
double b=10.1;
fesetround(FE_UPWARD);
assert(-((-a)b)!=ab);
}

Excessive optimization will remove the double negation. gcc has option -frounding-math to help with this. Intel's -fp-model strict is not quite as good (it fails for this example) but it helps (for instance it lets the code work if I write 1.1 and 10.1 instead of a and b in the assertion). With llvm-gcc or clang, this always fails at -O1.

A similar option for llvm would be very welcome, there are applications where rounding in the appropriate direction is crucial (precision and speed come far behind).

@llvmbot
Copy link
Member Author

llvmbot commented Sep 7, 2010

Lack of proper support for fp rounding has come up before, so I'm sure this bug
report is a duplicate. But I failed to find the earlier bug report...

@llvmbot
Copy link
Member Author

llvmbot commented Sep 9, 2010

The closest I could find is bug 3929 which asks for the FENV pragmas in llvm-gcc. Complete C99 fenv support in llvm+clang would be great, but an option like gcc's -frounding-math that disables some optimizations seems easier to achieve.

(I may have missed a real duplicate, I am not good at using bugzilla)

@lattner
Copy link
Collaborator

lattner commented Sep 27, 2010

*** Bug llvm/llvm-bugzilla-archive#8241 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Member Author

llvmbot commented Aug 7, 2011

Note that this impacts properly building packages like ppl 0.11.2 which expect functional -frounding-math support. Currently the only viable workaround when using the clang compilers is to build ppl with --disable-fpmath which reduces the functionality of the package.

@llvmbot
Copy link
Member Author

llvmbot commented Aug 7, 2011

Note that this impacts properly building packages like ppl 0.11.2 which expect
functional -frounding-math support. Currently the only viable workaround when
using the clang compilers is to build ppl with --disable-fpmath which reduces
the functionality of the package.

Does even -O0 fail?

In CGAL, every operation where rounding matters is done through macros which with clang go through volatile variables or asm statements to prevent broken optimizations. It is slow.

@lattner
Copy link
Collaborator

lattner commented May 26, 2012

*** Bug llvm/llvm-bugzilla-archive#10409 has been marked as a duplicate of this bug. ***

@lattner
Copy link
Collaborator

lattner commented May 26, 2012

*** Bug llvm/llvm-bugzilla-archive#12958 has been marked as a duplicate of this bug. ***

@asl
Copy link
Collaborator

asl commented Jun 3, 2013

*** Bug llvm/llvm-bugzilla-archive#16202 has been marked as a duplicate of this bug. ***

@efriedma-quic
Copy link
Collaborator

*** Bug llvm/llvm-bugzilla-archive#17088 has been marked as a duplicate of this bug. ***

@rnk
Copy link
Collaborator

rnk commented Jan 20, 2015

*** Bug llvm/llvm-bugzilla-archive#22271 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Member Author

llvmbot commented Mar 11, 2015

*** Bug llvm/llvm-bugzilla-archive#22873 has been marked as a duplicate of this bug. ***

@majnemer
Copy link
Mannequin

majnemer mannequin commented May 30, 2015

*** Bug llvm/llvm-bugzilla-archive#23707 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Member Author

llvmbot commented Dec 14, 2015

Test cases using rint() and nearbyint() from <math.h>
I noticed that problem when writing test-cases for rint() and nearbyint().

It looks to me like the compiler gets it right when the rounding mode is known at compile time. The attached test produced the following output for me:

kamikaze@AprilRyan> ./rint
Test fail:    FE_DOWNWARD rint(12.9) == 12.
Test fail:    FE_DOWNWARD rint(-12.1) == -13.
Test fail:    FE_DOWNWARD nearbyint(12.9) == 12.
Test fail:    FE_DOWNWARD nearbyint(-12.1)
Test success: FE_TONEAREST rint(12.4) == 12.
Test success: FE_TONEAREST rint(-12.4) == -12.
Test success: FE_TONEAREST nearbyint(12.4)
Test success: FE_TONEAREST nearbyint(-12.4) == -12.
Test fail:    FE_TOWARDZERO rint(12.9) == 12.
Test fail:    FE_TOWARDZERO rint(-12.9) == -12.
Test fail:    FE_TOWARDZERO nearbyint(12.9) == 12.
Test fail:    FE_TOWARDZERO nearbyint(-12.9) == -12.
Test fail:    FE_UPWARD rint(12.1) == 13.
Test fail:    FE_UPWARD rint(-12.9) == -12.
Test fail:    FE_UPWARD nearbyint(12.1) == 13.
Test fail:    FE_UPWARD nearbyint(-12.9) == -12.
Test success: FE_DOWNWARD rint(12.9) == 12.
Test success: FE_DOWNWARD rint(-12.1) == -13.
Test success: FE_DOWNWARD nearbyint(12.9) == 12.
Test success: FE_DOWNWARD nearbyint(-12.1)
Test success: FE_TONEAREST rint(12.4) == 12.
Test success: FE_TONEAREST rint(-12.4) == -12.
Test success: FE_TONEAREST nearbyint(12.4)e
Test success: FE_TONEAREST nearbyint(-12.4) == -12.
Test success: FE_TOWARDZERO rint(12.9) == 12.
Test success: FE_TOWARDZERO rint(-12.9) == -12.
Test success: FE_TOWARDZERO nearbyint(12.9) == 12.
Test success: FE_TOWARDZERO nearbyint(-12.9) == -12.
Test success: FE_UPWARD rint(12.1) == 13.
Test success: FE_UPWARD rint(-12.9) == -12.
Test success: FE_UPWARD nearbyint(12.1) == 13.
Test success: FE_UPWARD nearbyint(-12.9) == -12.

The first time the rounding mode comes from an array, the second time it's set to a literal value.

@llvmbot
Copy link
Member Author

llvmbot commented Dec 15, 2015

This violates both C99 and C++11/14.

COMPILER PRODUCES INCORRECT CODE. Why is this not a release breaker?

This is much worse than failing to compile valid code. This is compiling valid code into something unintended.

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Dec 15, 2015

This violates both C99 and C++11/14.

You are mistaken. Setting the rounding mode without using #pragma STDC FENV_ACCESS ON invokes undefined behavior. See C11 7.6.1/2. (This pragma does not exist in C++, so is unusable, but that's not our fault...)

Clang doesn't support that pragma, making this an unimplemented feature from C99, for which we will produce a warning or error. That's the subject of this enhancement request.

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Mar 14, 2016

*** Bug llvm/llvm-bugzilla-archive#26931 has been marked as a duplicate of this bug. ***

@delcypher
Copy link
Contributor

@delcypher
Copy link
Contributor

I just hit this issue and I've attached a test case (interval.c) that Clang mis-compiles.

It appears that when Clang is optimizing (-O3) it changes two separate fadd instructions that use a different rounding mode into a vectorized add using the wrong rounding modes.

Correct

  %25 = tail call i32 @&#8203;fegetround() #&#8203;8
  %26 = tail call i32 @&#8203;fesetround(i32 1024) #&#8203;9
  %27 = fadd float %3, %14
  %28 = tail call i32 @&#8203;fesetround(i32 2048) #&#8203;9
  %29 = fadd float %7, %18
  %30 = tail call i32 @&#8203;fesetround(i32 %25) #&#8203;9

Incorrect

  %23 = tail call i32 @&#8203;fegetround() #&#8203;8
  %24 = tail call i32 @&#8203;fesetround(i32 1024) #&#8203;9
  %25 = tail call i32 @&#8203;fesetround(i32 2048) #&#8203;9
  %26 = fadd <2 x float> %9, <float 5.000000e+06, float 5.000000e+06>

What surprised me a bit is that if I compile the test case with clang++ rather than clang the code is mis-compiled at -O2 rather than -O3.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 20, 2017

What's it going on?
As the pragma #pragma STDC FENV_ACCESS ON is hard to implement, how about a option to disable the optimization such as -frounding-math?
Either option or some ways to fix this bug?

@efriedma-quic
Copy link
Collaborator

*** Bug llvm/llvm-bugzilla-archive#39112 has been marked as a duplicate of this bug. ***

@efriedma-quic
Copy link
Collaborator

*** Bug llvm/llvm-bugzilla-archive#40954 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Member Author

llvmbot commented Mar 5, 2019

With regards to the resolution of Bug 40954, is this the intended duplicate bug? I'm confused as my reported bug report doesn't use rounding modes. It was a LICM miscompile as an fdiv (that can raise an exception) was moved with respect to its control-flow dependence.

@andykaylor
Copy link
Contributor

With regards to the resolution of Bug 40954, is this the intended duplicate
bug? I'm confused as my reported bug report doesn't use rounding modes. It
was a LICM miscompile as an fdiv (that can raise an exception) was moved
with respect to its control-flow dependence.

Hi, Eric.

This bug is serving as a catch all for a general class of problems related to the fact that LLVM treats FP operations as not having side effects. I'll explain why below. We are working on a general solution to this problem which when fully implemented will fix the problem that you reported.

The fact that LLVM treats FP operations as not having side effects is an intentional design decision. It enables better optimization of floating point code in the case where exceptions are not being unmasked and the status flags are not being checked, which we believe to be the case that the majority of clang and LLVM users care about.

If you look at the LLVM language reference you will find intrinsics defined for constrained versions of the FP operations. This is the approach we are taking to solve the side effect problem. From the perspective of LLVM IR, it is undefined behavior to unmask FP exceptions, read FP status flags, or change FP control flags when you are not using these intrinsics. (More precisely, I think it is the interaction between the FP environment and the non-constrained FP operations that is undefined.)

Similarly the C99 standard says that implementations are free to assume the default FP environment (round to nearest, no exceptions unmasked) and that the status flags will not be tested outside of scopes in which the FENV_ACCESS pragma state is "off". That's why your bug falls under the scope of this one.

@efriedma-quic
Copy link
Collaborator

*** Bug llvm/llvm-bugzilla-archive#44302 has been marked as a duplicate of this bug. ***

@vinc17fr
Copy link

vinc17fr commented Feb 7, 2020

Clang doesn't support that pragma,

Clang does not need to support it to behave correctly, see below.

making this an unimplemented feature from
C99, for which we will produce a warning or error. That's the subject of
this enhancement request.

There are 2 ways to conform to C99:

  1. Assume that the FENV_ACCESS pragma is always on, ignoring how the programmer sets it (but I suppose that this won't help here).

  2. Not define the optional macros related to <fenv.h>. See the C standard: "Each of the macros [...] is defined if and only if the implementation supports [...]". Since clang/llvm does not support these features (in the sense that it does not behave correctly, in many cases), such macros should not be defined.

@llvmbot
Copy link
Member Author

llvmbot commented Feb 7, 2020

Clang does not need to support it to behave correctly, see below.

Vincent, we are not lawyers trying to get our client off on a technicality, we want useful behavior for users.

There are 2 ways to conform to C99:

  1. Assume that the FENV_ACCESS pragma is always on, ignoring how the
    programmer sets it (but I suppose that this won't help here).

Ok, this one would help, it is gcc's -frounding-math -ftrapping-math (although it doesn't really work in gcc), but implementing it properly is likely almost as hard as supporting the pragma.

  1. Not define the optional macros related to <fenv.h>. See the C standard:
    "Each of the macros [...] is defined if and only if the implementation
    supports [...]". Since clang/llvm does not support these features (in the
    sense that it does not behave correctly, in many cases), such macros should
    not be defined.

This one would be counterproductive. We use rounded operations with clang every day, by placing optimization barriers (inline asm) around each such operation. Removing FE_UPWARD, while it would make the implementation conforming, would complicate things for us, we would have to reimplement the very platform-specific fesetround on every platform we support...

@vinc17fr
Copy link

vinc17fr commented Feb 7, 2020

Clang does not need to support it to behave correctly, see below.

Vincent, we are not lawyers trying to get our client off on a technicality,
we want useful behavior for users.

Entirely agreed. But returning incorrect results is definitively not useful for users, and actually very bad.

There are 2 ways to conform to C99:

  1. Assume that the FENV_ACCESS pragma is always on, ignoring how the
    programmer sets it (but I suppose that this won't help here).

Ok, this one would help, it is gcc's -frounding-math -ftrapping-math
(although it doesn't really work in gcc), but implementing it properly is
likely almost as hard as supporting the pragma.

AFAIK, -frounding-math works correctly in GCC if the user uses a single rounding mode in his code. At least, that's how it is documented.

  1. Not define the optional macros related to <fenv.h>. See the C standard:
    "Each of the macros [...] is defined if and only if the implementation
    supports [...]". Since clang/llvm does not support these features (in the
    sense that it does not behave correctly, in many cases), such macros should
    not be defined.

This one would be counterproductive. We use rounded operations with clang
every day, by placing optimization barriers (inline asm) around each such
operation.

Then perhaps this should be under the control of a compiler option, and the macros could be undefined by default at least in ISO C strict modes.

I doubt that every developer who needs directed rounding modes places optimization barriers around each FP operation. This is error prone (it is very easy to forget one) and makes the code more tedious to write and difficult to read.

Directed rounding modes are essentially there to get guaranteed results (interval arithmetic, error bounds...), so that even getting a slightly incorrect result is rather a critical issue.

@llvmbot
Copy link
Member Author

llvmbot commented Feb 7, 2020

AFAIK, -frounding-math works correctly in GCC if the user uses a single
rounding mode in his code.

Yes.

At least, that's how it is documented.

Gcc's manpage says "This option should be specified for programs that change the FP rounding mode dynamically" and doesn't mention this restriction of "use a single rounding mode", or I didn't look in the right place.

Then perhaps this should be under the control of a compiler option, and the
macros could be undefined by default at least in ISO C strict modes.

I don't care about C (only C++) so I won't fight that, but I'd rather spend energy moving epsilon closer to an implementation of the pragma than spend it removing useful, if misleading, macros.

I doubt that every developer who needs directed rounding modes places
optimization barriers around each FP operation.

They do if they use clang and care about correctness...

This is error prone (it is very easy to forget one)

With wrappers, it isn't that error prone.

and makes the code more tedious to write and difficult to read.

Yes, but that's an argument for implementing the feature, not removing the little that is there.

@andykaylor
Copy link
Contributor

FWIW, I think we're close to having this working on several architectures and any other architectures that want to enable it have several examples to follow for the backend changes that are needed.

The -fp-model=strict option puts us in a mode when FENV_ACCESS is on everywhere without the pragma. There's a patch up to support the pragma (https://reviews.llvm.org/D69272). There are problem a bunch of corner cases that we're missing, but for the most part I believe it is basically functional and in no place is it functionally worse than it was before. Try it out. File new bugs.

The one big caveat is that we're not currently doing anything to optimize the constrained FP code well. The way it's implemented basically turns all FP operations into black boxes and optimizations will need to be taught to handle them in light of the constraints. We have some ideas for that, but I don't think much, if anything, has been done.

@LebedevRI
Copy link
Member

*** Bug llvm/llvm-bugzilla-archive#45722 has been marked as a duplicate of this bug. ***

@zygoloid
Copy link
Mannequin

zygoloid mannequin commented Jul 22, 2020

*** Bug llvm/llvm-bugzilla-archive#46799 has been marked as a duplicate of this bug. ***

@lattner
Copy link
Collaborator

lattner commented Nov 27, 2021

mentioned in issue llvm/llvm-bugzilla-archive#8241

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 23, 2022
…Dylan-DPC

Document rounding for floating-point primitive operations and string parsing

The docs for floating point don't have much to say at present about either the precision of their results or rounding behaviour.

As I understand it[^1][^2], Rust doesn't support operating with non-default rounding directions, so we need only describe roundTiesToEven.

[^1]: rust-lang#41753 (comment)
[^2]: llvm/llvm-project#8472 (comment)

This PR makes a start by documenting that for primitive operations and `from_str()`.
workingjubilee pushed a commit to tcdi/postgrestd that referenced this issue Sep 15, 2022
Document rounding for floating-point primitive operations and string parsing

The docs for floating point don't have much to say at present about either the precision of their results or rounding behaviour.

As I understand it[^1][^2], Rust doesn't support operating with non-default rounding directions, so we need only describe roundTiesToEven.

[^1]: rust-lang/rust#41753 (comment)
[^2]: llvm/llvm-project#8472 (comment)

This PR makes a start by documenting that for primitive operations and `from_str()`.
@arsenm arsenm added the floating-point Floating-point math label Aug 11, 2023
augusto2112 added a commit to augusto2112/llvm-project that referenced this issue Mar 25, 2024
[lldb] Support frame variable for generic types in embedded Swift
freebsd-git pushed a commit to freebsd/freebsd-src that referenced this issue Jun 12, 2024
LLVM bugzilla bug 8100 became issue #8472 with the migration to GitHub.

llvm/llvm-project#8472
emaste added a commit to emaste/freebsd that referenced this issue Aug 1, 2024
LLVM bugzilla bug 8100 became issue #8472 with the migration to GitHub.

llvm/llvm-project#8472
(cherry picked from commit 92927b8)
freebsd-git pushed a commit to freebsd/freebsd-src that referenced this issue Aug 6, 2024
LLVM bugzilla bug 8100 became issue #8472 with the migration to GitHub.

llvm/llvm-project#8472
(cherry picked from commit 92927b8)
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this issue Sep 6, 2024
LLVM bugzilla bug 8100 became issue #8472 with the migration to GitHub.

llvm/llvm-project#8472
freebsd-git pushed a commit to freebsd/freebsd-src that referenced this issue Oct 1, 2024
LLVM bugzilla bug 8100 became issue #8472 with the migration to GitHub.

llvm/llvm-project#8472
(cherry picked from commit 92927b8)
(cherry picked from commit 6cd4450)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" floating-point Floating-point math
Projects
None yet
Development

No branches or pull requests

10 participants