diff mbox

Turn -fexcess-precision=fast on when in -ffast-math

Message ID 1482236061-32484-1-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh Dec. 20, 2016, 12:14 p.m. UTC
On Tue, Dec 20, 2016 at 11:48:26AM +0100, Richard Biener wrote:
> On Mon, Dec 19, 2016 at 6:58 PM, James Greenhalgh
> <james.greenhalgh@arm.com> wrote:
> >
> >> On Thu, Dec 8, 2016 at 10:44 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> >> > Hello!
> >> >
> >> > Attached patch fixes fall-out from excess-precision improvements
> >> > patch. As shown in the PR, the code throughout the compiler assumes
> >> > FLAG_PRECISION_FAST when flag_unsafe_math_optimizations flag is in
> >> > effect. The patch puts back two lines, removed by excess-precision
> >> > improvements patch.
> >> >
> >> > 2016-12-08  Uros Bizjak  <ubizjak@gmail.com>
> >> >
> >> >     PR middle-end/78738
> >> >     * toplev.c (init_excess_precision): Initialize flag_excess_precision
> >> >     to EXCESS_PRECISION_FAST for flag_unsafe_math_optimizations.
> >> >
> >> > testsuite/ChangeLog:
> >> >
> >> > 2016-12-08  Uros Bizjak  <ubizjak@gmail.com>
> >> >
> >> >     PR middle-end/78738
> >> >     * gcc.target/i386/pr78738.c: New test.
> >> >
> >> > Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
> >> >
> >> > OK for mainline?
> >>
> >> Hmm, I think it belongs to set_unsafe_math_optimization_flags instead
> >> (and be consistent if -fexcess-precision was manually specified).
> >
> > I think it would be better if this were implied by -ffast-math/-Ofast
> > than by -funsafe-math-optimizations . That's what I've implemented here,
> > and tagged the option as SetByCombined to allow us to honour what the
> > user requests.
> >
> > This should give us the behaviour you were looking for Uros.
> >
> > I've bootstrapped and tested the behaviour on x86_64, and I've hacked up
> > the AArch64 backend to validate that we're setting the flag in the right
> > circumstances (but that meant changing the AArch64 behaviour, so isn't
> > something we'd want on trunk, and therefore I can't write a testcase for
> > this patch).
> >
> > OK?
>
> Looks good to me, but please also adjust invoke.texi to list -fexcess-precision
> as affected by -ffast-math.
>
> Ok with that change.

Thanks, I've modified the affected portions of the documentation.

As I've made a few mistakes in this area recently, I'll hold off on
committing the patch until these documentation changes have been looked
at by Sandra.

OK?

Thanks,
James

---
2016-12-20  James Greenhalgh  <james.greenhalghj@arm.com>

	* common.opt (excess_precision): Tag as SetByCombined.
	* opts.c (set_fast_math_flags): Also set
	flag_excess_precision_cmdline.
	(fast_math_flags_set_p): Also check flag_excess_precision_cmdline.
	* doc/invoke.texi (-fexcess-precision): Drop text saying the
	option has no effect under -ffast-math, make it clear that
	-ffast-math will cause -fexcess-precision=fast by default even for
	standards compliant modes.
	(-ffast-math): Document that this sets -fexcess-precision=fast.

Comments

Sandra Loosemore Dec. 20, 2016, 4:05 p.m. UTC | #1
On 12/20/2016 05:14 AM, James Greenhalgh wrote:
>
> On Tue, Dec 20, 2016 at 11:48:26AM +0100, Richard Biener wrote:
>> On Mon, Dec 19, 2016 at 6:58 PM, James Greenhalgh
>> <james.greenhalgh@arm.com> wrote:
>>>
>>>> On Thu, Dec 8, 2016 at 10:44 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>>> Hello!
>>>>>
>>>>> Attached patch fixes fall-out from excess-precision improvements
>>>>> patch. As shown in the PR, the code throughout the compiler assumes
>>>>> FLAG_PRECISION_FAST when flag_unsafe_math_optimizations flag is in
>>>>> effect. The patch puts back two lines, removed by excess-precision
>>>>> improvements patch.
>>>>>
>>>>> 2016-12-08  Uros Bizjak  <ubizjak@gmail.com>
>>>>>
>>>>>      PR middle-end/78738
>>>>>      * toplev.c (init_excess_precision): Initialize flag_excess_precision
>>>>>      to EXCESS_PRECISION_FAST for flag_unsafe_math_optimizations.
>>>>>
>>>>> testsuite/ChangeLog:
>>>>>
>>>>> 2016-12-08  Uros Bizjak  <ubizjak@gmail.com>
>>>>>
>>>>>      PR middle-end/78738
>>>>>      * gcc.target/i386/pr78738.c: New test.
>>>>>
>>>>> Patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>>>>>
>>>>> OK for mainline?
>>>>
>>>> Hmm, I think it belongs to set_unsafe_math_optimization_flags instead
>>>> (and be consistent if -fexcess-precision was manually specified).
>>>
>>> I think it would be better if this were implied by -ffast-math/-Ofast
>>> than by -funsafe-math-optimizations . That's what I've implemented here,
>>> and tagged the option as SetByCombined to allow us to honour what the
>>> user requests.
>>>
>>> This should give us the behaviour you were looking for Uros.
>>>
>>> I've bootstrapped and tested the behaviour on x86_64, and I've hacked up
>>> the AArch64 backend to validate that we're setting the flag in the right
>>> circumstances (but that meant changing the AArch64 behaviour, so isn't
>>> something we'd want on trunk, and therefore I can't write a testcase for
>>> this patch).
>>>
>>> OK?
>>
>> Looks good to me, but please also adjust invoke.texi to list -fexcess-precision
>> as affected by -ffast-math.
>>
>> Ok with that change.
>
> Thanks, I've modified the affected portions of the documentation.
>
> As I've made a few mistakes in this area recently, I'll hold off on
> committing the patch until these documentation changes have been looked
> at by Sandra.
>
> OK?

I only have one tiny nit, in this snippet:

>  semantics apply without excess precision, and in the latter, rounding
>  is unpredictable.
>
> +
>  @item -ffast-math
>  @opindex ffast-math
>  Sets the options @option{-fno-math-errno}, @option{-funsafe-math-optimizations},

Please don't introduce unnecessary whitespace changes, or mix them with 
changes to technical content.  The doc parts are OK with that repaired.

-Sandra
diff mbox

Patch

diff --git a/gcc/common.opt b/gcc/common.opt
index de06844..6ebaf9c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1317,7 +1317,7 @@  Common Report Var(flag_expensive_optimizations) Optimization
 Perform a number of minor, expensive optimizations.
 
 fexcess-precision=
-Common Joined RejectNegative Enum(excess_precision) Var(flag_excess_precision_cmdline) Init(EXCESS_PRECISION_DEFAULT)
+Common Joined RejectNegative Enum(excess_precision) Var(flag_excess_precision_cmdline) Init(EXCESS_PRECISION_DEFAULT) SetByCombined
 -fexcess-precision=[fast|standard]	Specify handling of excess floating-point precision.
 
 Enum
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b729964..8c5308e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -9109,21 +9109,23 @@  both casts and assignments cause values to be rounded to their
 semantic types (whereas @option{-ffloat-store} only affects
 assignments).  This option is enabled by default for C if a strict
 conformance option such as @option{-std=c99} is used.
+@option{-ffast-math} enables @option{-fexcess-precision=fast} by default
+regardless of whether a strict conformance option is used.
 
 @opindex mfpmath
 @option{-fexcess-precision=standard} is not implemented for languages
-other than C, and has no effect if
-@option{-funsafe-math-optimizations} or @option{-ffast-math} is
-specified.  On the x86, it also has no effect if @option{-mfpmath=sse}
+other than C.  On the x86, it has no effect if @option{-mfpmath=sse}
 or @option{-mfpmath=sse+387} is specified; in the former case, IEEE
 semantics apply without excess precision, and in the latter, rounding
 is unpredictable.
 
+
 @item -ffast-math
 @opindex ffast-math
 Sets the options @option{-fno-math-errno}, @option{-funsafe-math-optimizations},
 @option{-ffinite-math-only}, @option{-fno-rounding-math},
-@option{-fno-signaling-nans} and @option{-fcx-limited-range}.
+@option{-fno-signaling-nans}, @option{-fcx-limited-range} and
+@option{-fexcess-precision=fast}.
 
 This option causes the preprocessor macro @code{__FAST_MATH__} to be defined.
 
diff --git a/gcc/opts.c b/gcc/opts.c
index 890da03..5844190 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2342,6 +2342,10 @@  set_fast_math_flags (struct gcc_options *opts, int set)
     opts->x_flag_errno_math = !set;
   if (set)
     {
+      if (opts->frontend_set_flag_excess_precision_cmdline
+	  == EXCESS_PRECISION_DEFAULT)
+	opts->x_flag_excess_precision_cmdline
+	  = set ? EXCESS_PRECISION_FAST : EXCESS_PRECISION_DEFAULT;
       if (!opts->frontend_set_flag_signaling_nans)
 	opts->x_flag_signaling_nans = 0;
       if (!opts->frontend_set_flag_rounding_math)
@@ -2374,7 +2378,9 @@  fast_math_flags_set_p (const struct gcc_options *opts)
 	  && opts->x_flag_unsafe_math_optimizations
 	  && opts->x_flag_finite_math_only
 	  && !opts->x_flag_signed_zeros
-	  && !opts->x_flag_errno_math);
+	  && !opts->x_flag_errno_math
+	  && opts->x_flag_excess_precision_cmdline
+	     == EXCESS_PRECISION_FAST);
 }
 
 /* Return true iff flags are set as if -ffast-math but using the flags stored