diff mbox

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

Message ID 1482170286-35068-1-git-send-email-james.greenhalgh@arm.com
State Superseded
Headers show

Commit Message

James Greenhalgh Dec. 19, 2016, 5:58 p.m. UTC
> 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?

Thanks,
James

---
2016-12-19  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.

Comments

Richard Biener Dec. 20, 2016, 10:48 a.m. UTC | #1
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.
Richard.

> Thanks,

> James

>

> ---

> 2016-12-19  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.

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