diff mbox series

Disable -ftrapping-math by default

Message ID DB6PR0801MB205342DB187C144927AC5FBB832E0@DB6PR0801MB2053.eurprd08.prod.outlook.com
State New
Headers show
Series Disable -ftrapping-math by default | expand

Commit Message

Wilco Dijkstra Nov. 16, 2017, 2:33 p.m. UTC
GCC currently defaults to -ftrapping-math.  This is supposed to generate
code for correct user-visible traps and FP status flags.

However it doesn't work as expected since it doesn't block any floating
point optimizations.  For example it continues to perform CSE, moves FP
operations across calls, moves FP operations out of loops, constant folds
and removes dead floating point operations that cause exceptions.

Given the majority of code doesn't contain user trap handlers or inspects
FP status flags, there is no point in enabling it even if it worked as expected.

Simple case that should cause a FP exception:

void f(void)
{
  0.0 / 0.0;
}

Compiles to:

f:
	ret

OK for commit?

2017-11-16  Wilco Dijkstra  <wdijkstr@arm.com>

	* common.opt (ftrapping-math): Change default to 0.
	* doc/invoke.texi (-ftrapping-math): Update documentation.
--

Comments

Richard Biener Nov. 16, 2017, 2:48 p.m. UTC | #1
On Thu, Nov 16, 2017 at 3:33 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
> GCC currently defaults to -ftrapping-math.  This is supposed to generate
> code for correct user-visible traps and FP status flags.
>
> However it doesn't work as expected since it doesn't block any floating
> point optimizations.  For example it continues to perform CSE, moves FP
> operations across calls, moves FP operations out of loops, constant folds
> and removes dead floating point operations that cause exceptions.
>
> Given the majority of code doesn't contain user trap handlers or inspects
> FP status flags, there is no point in enabling it even if it worked as expected.
>
> Simple case that should cause a FP exception:
>
> void f(void)
> {
>   0.0 / 0.0;
> }
>
> Compiles to:
>
> f:
>         ret

We are generally not preserving traps but we guard any transform that
might introduce traps with -ftrapping-math.  That's similar to how we treat
-ftrapv and pointer dereferences.

We're mitigating the "bad" effect of the -ftrapping-math default
by defaulting to -fno-signalling-nans.

If it doesn't block any optimizations what's the point of the patch?

Richard.

> OK for commit?
>
> 2017-11-16  Wilco Dijkstra  <wdijkstr@arm.com>
>
>         * common.opt (ftrapping-math): Change default to 0.
>         * doc/invoke.texi (-ftrapping-math): Update documentation.
> --
>
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 1bb87353f760d7c60c39de8b9de4311c1ec3d892..59940c64356964f8f9b9d842ad3f1a1c02548bab 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2470,7 +2470,7 @@ generate them instead of using descriptors.
>  ; (user-visible) trap.  This is the case, for example, in nonstop
>  ; IEEE 754 arithmetic.
>  ftrapping-math
> -Common Report Var(flag_trapping_math) Init(1) Optimization SetByCombined
> +Common Report Var(flag_trapping_math) Init(0) Optimization SetByCombined
>  Assume floating-point operations can trap.
>
>  ftrapv
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index 74c33ea35b9f320b419a3417e6007d2391536f1b..3673b34b3b7f7b57cfa6375b5316f9f282a9e9bb 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -9479,20 +9479,21 @@ This option implies that the sign of a zero result isn't significant.
>
>  The default is @option{-fsigned-zeros}.
>
> -@item -fno-trapping-math
> -@opindex fno-trapping-math
> -Compile code assuming that floating-point operations cannot generate
> +@item -ftrapping-math
> +@opindex ftrapping-math
> +Compile code assuming that floating-point operations can generate
>  user-visible traps.  These traps include division by zero, overflow,
> -underflow, inexact result and invalid operation.  This option requires
> -that @option{-fno-signaling-nans} be in effect.  Setting this option may
> -allow faster code if one relies on ``non-stop'' IEEE arithmetic, for example.
> +underflow, inexact result and invalid operation.
>
> -This option should never be turned on by any @option{-O} option since
> -it can result in incorrect output for programs that depend on
> -an exact implementation of IEEE or ISO rules/specifications for
> -math functions.
> +Note this option has only been partially implemented and does not work
> +as expected.  For example @option{-ftrapping-math} performs floating
> +point optimizations such as loop invariant motion, constant folding
> +and scheduling across function calls which have user-visible effects
> +on FP exception flags.
> +
> +This option is turned on when using @option{-fsignaling-nans}.
>
> -The default is @option{-ftrapping-math}.
> +The default is @option{-fno-trapping-math}.
>
>  @item -frounding-math
>  @opindex frounding-math
Wilco Dijkstra Nov. 16, 2017, 3:48 p.m. UTC | #2
Richard Biener wrote:

> We are generally not preserving traps but we guard any transform that
> might introduce traps with -ftrapping-math.  That's similar to how we treat
> -ftrapv and pointer dereferences.

Right. It appears it's mostly concerned about division - if it is about division
by zero aborting like a null pointer reference then maybe it should be renamed
to -ftrap-fp-division-by-zero? Are there targets that abort like this?

> We're mitigating the "bad" effect of the -ftrapping-math default
> by defaulting to -fno-signalling-nans.
>
> If it doesn't block any optimizations what's the point of the patch?

It certainly blocks some optimizations, I noticed it affects the division reciprocal
optimization.

Wilco
Joseph Myers Nov. 16, 2017, 11 p.m. UTC | #3
I'd expect (considering the whole GNU toolchain, not just GCC in 
isolation) this to require a corresponding glibc change to build with 
-ftrapping-math (added alongside the use of -frounding-math - adding the 
option is of course save independently of the GCC change).

Also, GCC tests involving floating-point exceptions need to use 
-ftrapping-math with this patch.  That probably means making 
add_options_for_ieee always use -ftrapping-math, and making sure all tests 
that use the fenv_exceptions effective target also use dg-add-options 
ieee.
Marc Glisse Nov. 16, 2017, 11:10 p.m. UTC | #4
On Thu, 16 Nov 2017, Richard Biener wrote:

> On Thu, Nov 16, 2017 at 3:33 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com> wrote:
>> GCC currently defaults to -ftrapping-math.  This is supposed to generate
>> code for correct user-visible traps and FP status flags.
>>
>> However it doesn't work as expected since it doesn't block any floating
>> point optimizations.  For example it continues to perform CSE, moves FP
>> operations across calls, moves FP operations out of loops, constant folds
>> and removes dead floating point operations that cause exceptions.
>>
>> Given the majority of code doesn't contain user trap handlers or inspects
>> FP status flags, there is no point in enabling it even if it worked as expected.
>>
>> Simple case that should cause a FP exception:
>>
>> void f(void)
>> {
>>   0.0 / 0.0;
>> }
>>
>> Compiles to:
>>
>> f:
>>         ret
>
> We are generally not preserving traps but we guard any transform that
> might introduce traps with -ftrapping-math.  That's similar to how we treat
> -ftrapv and pointer dereferences.

Joseph seems to have a different opinion in PR 53805. Splitting the option 
(one strict version that preserves floating point status changes, one weak 
version that only avoids introducing new traps but may remove existing 
ones) would help clarify the situation.

> We're mitigating the "bad" effect of the -ftrapping-math default
> by defaulting to -fno-signalling-nans.
>
> If it doesn't block any optimizations what's the point of the patch?
>
> Richard.
>
>> OK for commit?
>>
>> 2017-11-16  Wilco Dijkstra  <wdijkstr@arm.com>

You could mention PR 54192 here.

>>         * common.opt (ftrapping-math): Change default to 0.
>>         * doc/invoke.texi (-ftrapping-math): Update documentation.

I am surprised that no testsuite change is needed.
Richard Biener Nov. 17, 2017, 11:52 a.m. UTC | #5
On Fri, Nov 17, 2017 at 12:10 AM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Thu, 16 Nov 2017, Richard Biener wrote:
>
>> On Thu, Nov 16, 2017 at 3:33 PM, Wilco Dijkstra <Wilco.Dijkstra@arm.com>
>> wrote:
>>>
>>> GCC currently defaults to -ftrapping-math.  This is supposed to generate
>>> code for correct user-visible traps and FP status flags.
>>>
>>> However it doesn't work as expected since it doesn't block any floating
>>> point optimizations.  For example it continues to perform CSE, moves FP
>>> operations across calls, moves FP operations out of loops, constant folds
>>> and removes dead floating point operations that cause exceptions.
>>>
>>> Given the majority of code doesn't contain user trap handlers or inspects
>>> FP status flags, there is no point in enabling it even if it worked as
>>> expected.
>>>
>>> Simple case that should cause a FP exception:
>>>
>>> void f(void)
>>> {
>>>   0.0 / 0.0;
>>> }
>>>
>>> Compiles to:
>>>
>>> f:
>>>         ret
>>
>>
>> We are generally not preserving traps but we guard any transform that
>> might introduce traps with -ftrapping-math.  That's similar to how we
>> treat
>> -ftrapv and pointer dereferences.
>
>
> Joseph seems to have a different opinion in PR 53805. Splitting the option
> (one strict version that preserves floating point status changes, one weak
> version that only avoids introducing new traps but may remove existing ones)
> would help clarify the situation.

Maybe.  Though we certainly don't handle the situation consistently.

>> We're mitigating the "bad" effect of the -ftrapping-math default
>> by defaulting to -fno-signalling-nans.
>>
>> If it doesn't block any optimizations what's the point of the patch?
>>
>> Richard.
>>
>>> OK for commit?
>>>
>>> 2017-11-16  Wilco Dijkstra  <wdijkstr@arm.com>
>
>
> You could mention PR 54192 here.
>
>>>         * common.opt (ftrapping-math): Change default to 0.
>>>         * doc/invoke.texi (-ftrapping-math): Update documentation.
>
>
> I am surprised that no testsuite change is needed.

Indeed.  Would be a good time to add some tests then, esp. for the cases
that we should now optimize by default (still wondering which case
this should be).

Richard.

> --
> Marc Glisse
diff mbox series

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index 1bb87353f760d7c60c39de8b9de4311c1ec3d892..59940c64356964f8f9b9d842ad3f1a1c02548bab 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2470,7 +2470,7 @@  generate them instead of using descriptors.
 ; (user-visible) trap.  This is the case, for example, in nonstop
 ; IEEE 754 arithmetic.
 ftrapping-math
-Common Report Var(flag_trapping_math) Init(1) Optimization SetByCombined
+Common Report Var(flag_trapping_math) Init(0) Optimization SetByCombined
 Assume floating-point operations can trap.
 
 ftrapv
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 74c33ea35b9f320b419a3417e6007d2391536f1b..3673b34b3b7f7b57cfa6375b5316f9f282a9e9bb 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9479,20 +9479,21 @@  This option implies that the sign of a zero result isn't significant.
 
 The default is @option{-fsigned-zeros}.
 
-@item -fno-trapping-math
-@opindex fno-trapping-math
-Compile code assuming that floating-point operations cannot generate
+@item -ftrapping-math
+@opindex ftrapping-math
+Compile code assuming that floating-point operations can generate
 user-visible traps.  These traps include division by zero, overflow,
-underflow, inexact result and invalid operation.  This option requires
-that @option{-fno-signaling-nans} be in effect.  Setting this option may
-allow faster code if one relies on ``non-stop'' IEEE arithmetic, for example.
+underflow, inexact result and invalid operation.
 
-This option should never be turned on by any @option{-O} option since
-it can result in incorrect output for programs that depend on
-an exact implementation of IEEE or ISO rules/specifications for
-math functions.
+Note this option has only been partially implemented and does not work
+as expected.  For example @option{-ftrapping-math} performs floating
+point optimizations such as loop invariant motion, constant folding
+and scheduling across function calls which have user-visible effects
+on FP exception flags.
+
+This option is turned on when using @option{-fsignaling-nans}.
 
-The default is @option{-ftrapping-math}.
+The default is @option{-fno-trapping-math}.
 
 @item -frounding-math
 @opindex frounding-math