This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: fast_math_flags_set_p vs. set_fast_math_flags inconsistency?


Joseph Myers wrote:
> On Mon, 27 Jan 2020, Ulrich Weigand wrote:
> > I see.  I guess that makes me wonder what -fno-fast-math *ever* does
> > (except canceling a -ffast-math earlier on the command line).  Looking
> > at the current code, -fno-fast-math (just like -ffast-math) only ever
> > sets flags whose default is not overridden on the command line, but
> > then it always sets them to their default value!
> 
> As a general principle, more specific flags take precedence over less 
> specific ones, regardless of the command-line order.  So it's correct for 
> -ffast-math and -fno-fast-math not to do anything with a flag that was 
> explicitly overridden by the user (modulo any issues where a particular 
> combination of flags is unsupported by GCC, as with the 
> "%<-fassociative-math%> disabled; other options take precedence" case in 
> toplev.c).

Sure, I agree with the intended behavior as you describe.  I was just 
confused under what circumstances set_fast_math_flags with set == 0
ever does anything.  However, I've now understood that this is required
e.g. for the -Ofast -fno-fast-math case (this seems to be the only case
where set_fast_math_flags is called twice, first with set == 1 and then
with set == 0).

And indeed, in this case we see the effect of not resetting the
flag_cx_limited_range (on x86_64):

$ gcc -E -dD -x c /dev/null -std=c99 | grep GCC_IEC
#define __GCC_IEC_559 2
#define __GCC_IEC_559_COMPLEX 2
$ gcc -E -dD -x c /dev/null -std=c99 -Ofast | grep GCC_IEC
#define __GCC_IEC_559 0
#define __GCC_IEC_559_COMPLEX 0
$ gcc -E -dD -x c /dev/null -std=c99 -Ofast -fno-fast-math | grep GCC_IEC
#define __GCC_IEC_559 2
#define __GCC_IEC_559_COMPLEX 0

Note how __GCC_IEC_559_COMPLEX is not properly reset.

Similarly, we see the effect of not resetting flag_excess_precision
(only on i386):

$ gcc -E -dD -x c /dev/null -m32 -std=c99 | grep GCC_IEC
#define __GCC_IEC_559 2
#define __GCC_IEC_559_COMPLEX 2
$ gcc -E -dD -x c /dev/null -m32 -std=c99 -Ofast | grep GCC_IEC
#define __GCC_IEC_559 0
#define __GCC_IEC_559_COMPLEX 0
$ gcc -E -dD -x c /dev/null -m32 -std=c99 -Ofast -fno-fast-math | grep GCC_IEC
#define __GCC_IEC_559 0
#define __GCC_IEC_559_COMPLEX 0

Note how __GCC_IEC_559 is not properly reset either.

So it seems to me that indeed both of these should be reset
in the !set case of set_fast_math_flags.

In addition, we've already agreed that these three flags should be
checked in fast_math_flags_set_p, where they are currently missing:

  flag_associative_math
  flag_reciprocal_math
  flag_cx_limited_range

Finally, this remaining piece of code in set_fast_math_flags:

 if (set)
   {
     if (!opts->frontend_set_flag_signaling_nans)
       opts->x_flag_signaling_nans = 0;
     if (!opts->frontend_set_flag_rounding_math)
       opts->x_flag_rounding_math = 0;
   }

seems always a no-op since it only ever sets the flags to their
default value when they still must have that same default value.

For clarity, I'd suggest to either remove this code or replace
it with a comment.


The following patch (not yet fully tested) implements the above changes.
Note that this now brings the set of flags set and reset by
set_fast_math_flags completely in sync with the set of flags
tested in fast_math_flags_set_p.

Does this make sense to you?

Thanks,
Ulrich


ChangeLog:

	* opts.c (set_fast_math_flags): In the !set case, also reset
	x_flag_cx_limited_range and x_flag_excess_precision.  Remove dead
	code resetting x_flag_signaling_nans and x_flag_rounding_math.
	(fast_math_flags_set_p): Also test x_flag_cx_limited_range,
	x_flag_associative_math, and x_flag_reciprocal_math.


diff --git a/gcc/opts.c b/gcc/opts.c
index 7affeb4..4452793 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -2850,18 +2850,14 @@ set_fast_math_flags (struct gcc_options *opts, int set)
     opts->x_flag_finite_math_only = set;
   if (!opts->frontend_set_flag_errno_math)
     opts->x_flag_errno_math = !set;
-  if (set)
-    {
-      if (opts->frontend_set_flag_excess_precision == EXCESS_PRECISION_DEFAULT)
-	opts->x_flag_excess_precision
-	  = 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)
-	opts->x_flag_rounding_math = 0;
-      if (!opts->frontend_set_flag_cx_limited_range)
-	opts->x_flag_cx_limited_range = 1;
-    }
+  if (!opts->frontend_set_flag_cx_limited_range)
+    opts->x_flag_cx_limited_range = set;
+  if (!opts->frontend_set_flag_excess_precision)
+    opts->x_flag_excess_precision
+      = set ? EXCESS_PRECISION_FAST : EXCESS_PRECISION_DEFAULT;
+
+  // -ffast-math should also reset -fsignaling-nans and -frounding-math,
+  // but since those are off by default, there's nothing to do for now.
 }
 
 /* When -funsafe-math-optimizations is set the following
@@ -2884,10 +2880,13 @@ bool
 fast_math_flags_set_p (const struct gcc_options *opts)
 {
   return (!opts->x_flag_trapping_math
+	  && !opts->x_flag_signed_zeros
+	  && opts->x_flag_associative_math
+	  && opts->x_flag_reciprocal_math
 	  && 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_cx_limited_range
 	  && opts->x_flag_excess_precision == EXCESS_PRECISION_FAST);
 }
 

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]