search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

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

Open Quuxplusone opened 14 years ago

Quuxplusone commented 14 years ago
Bugzilla Link PR8100
Status NEW
Importance P enhancement
Reported by Marc Glisse (marc.glisse@normalesup.org)
Reported on 2010-09-07 11:39:35 -0700
Last modified on 2021-08-05 09:07:37 -0700
Version trunk
Hardware PC Linux
CC andrew.kaylor@intel.com, baldrick@free.fr, d.grellscheid+llvm@gmail.com, dan@su-root.co.uk, dimitry@andric.com, emaste@freebsd.org, eschweitz@nvidia.com, fwage73@gmail.com, gyllenllvm@gmail.com, howarth.mailing.lists@gmail.com, irving@naml.us, ismail@i10z.com, kamikaze@bsdforen.de, kcc@google.com, laurent.rineaullvm@nsup.org, Laurent.Rineaullvm_bugs@normalesup.org, llvm-bugs@lists.llvm.org, llvm@sunfishcode.online, paumard@users.sourceforge.net, richard-llvm@metafoo.co.uk, rjarrett@mathworks.com, rnk@google.com, spatel+llvm@rotateright.com, tavianator@tavianator.com, tydeman@tybor.com, vincent-llvm@vinc17.net, vonosmas@gmail.com, vz-llvm@zeitlins.org, westion717@gmail.com, yabinc@google.com, yottui@yahoo.co.jp, yuanfang.chen@sony.com, zamazan4ik@tut.by
Fixed by commit(s)
Attachments rint.c (2763 bytes, application/octet-stream)
interval.c (2933 bytes, text/x-csrc)
Blocks
Blocked by
See also PR24807, PR20358, PR17854, PR26931, PR45722 
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)!=a*b);
}

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).
Quuxplusone commented 14 years ago 
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...
Quuxplusone commented 14 years ago

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)

Quuxplusone commented 14 years ago

_Bug 8241 has been marked as a duplicate of this bug._

Quuxplusone commented 13 years ago

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.

Quuxplusone commented 13 years ago 
(In reply to comment #4)
> 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.
Quuxplusone commented 12 years ago

_Bug 10409 has been marked as a duplicate of this bug._

Quuxplusone commented 12 years ago

_Bug 12958 has been marked as a duplicate of this bug._

Quuxplusone commented 11 years ago

_Bug 16202 has been marked as a duplicate of this bug._

Quuxplusone commented 11 years ago

_Bug 17088 has been marked as a duplicate of this bug._

Quuxplusone commented 9 years ago

_Bug 22271 has been marked as a duplicate of this bug._

Quuxplusone commented 9 years ago

_Bug 22873 has been marked as a duplicate of this bug._

Quuxplusone commented 9 years ago

_Bug 23707 has been marked as a duplicate of this bug._

Quuxplusone commented 8 years ago

Attached rint.c (2763 bytes, application/octet-stream): Test cases using rint() and nearbyint() from

Quuxplusone commented 8 years ago

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.

Quuxplusone commented 8 years ago 
(In reply to comment #14)
> 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 <cfenv> 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.
Quuxplusone commented 8 years ago

_Bug 26931 has been marked as a duplicate of this bug._

Quuxplusone commented 7 years ago

Attached interval.c (2933 bytes, text/x-csrc): A test case that is mis-compiled by clang

Quuxplusone commented 7 years ago

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 @fegetround() #8
  %26 = tail call i32 @fesetround(i32 1024) #9
  %27 = fadd float %3, %14
  %28 = tail call i32 @fesetround(i32 2048) #9
  %29 = fadd float %7, %18
  %30 = tail call i32 @fesetround(i32 %25) #9

Incorrect

  %23 = tail call i32 @fegetround() #8
  %24 = tail call i32 @fesetround(i32 1024) #9
  %25 = tail call i32 @fesetround(i32 2048) #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.

Quuxplusone commented 7 years ago

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?

Quuxplusone commented 6 years ago

_Bug 39112 has been marked as a duplicate of this bug._

Quuxplusone commented 5 years ago

_Bug 40954 has been marked as a duplicate of this bug._

Quuxplusone commented 5 years ago

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.

Quuxplusone commented 5 years ago 
(In reply to Eric Schweitz from comment #22)
> 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.
Quuxplusone commented 4 years ago

_Bug 44302 has been marked as a duplicate of this bug._

Quuxplusone commented 4 years ago 
(In reply to Richard Smith from comment #15)
> 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.
Quuxplusone commented 4 years ago 
(In reply to Vincent Lefevre from comment #25)
> 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.

> 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.

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...
Quuxplusone commented 4 years ago 
(In reply to Marc Glisse from comment #26)
> (In reply to Vincent Lefevre from comment #25)
> > 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.

> > 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.
>
> 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.
Quuxplusone commented 4 years ago 
(In reply to Vincent Lefevre from comment #27)
> 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.
Quuxplusone commented 4 years ago

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.

Quuxplusone commented 4 years ago

_Bug 45722 has been marked as a duplicate of this bug._

Quuxplusone commented 4 years ago

_Bug 46799 has been marked as a duplicate of this bug._