diff mbox series

[v2] Fix -ffast-math flags handling inconsistencies

Message ID 20200211184332.0968DD80313@oc3748833570.ibm.com
State New
Headers show
Series [v2] Fix -ffast-math flags handling inconsistencies | expand

Commit Message

Ulrich Weigand Feb. 11, 2020, 6:43 p.m. UTC
Hello,

this second version of the patch incorporates all changes discussed
in the original mail thread starting here:
https://gcc.gnu.org/ml/gcc-patches/2020-01/msg02097.html

The patch now implements the following set of changes:

1. If a component flag of -ffast-math (or -funsafe-math-optimizations)
   is explicitly set (or reset) on the command line, this should override
   any implicit change due to -f(no-)fast-math, no matter in which order
   the flags come on the command line.  This change affects all flags.

2. Any component flag modified from its default by -ffast-math should
   be reset to the default by -fno-fast-math.  This was previously
   not done for the following flags:
      -fcx-limited-range
      -fexcess-precision=

3. Once -ffinite-math-only is true, the -f(no-)signaling-nans flag has
   no meaning (if we have no NaNs at all, it does not matter whether
   there is a difference between quiet and signaling NaNs).  Therefore,
   it does not make sense for -ffast-math to imply -fno-signaling-nans.
   This is also a documentation change.

4. -ffast-math is documented to imply -fno-rounding-math, however the
   latter setting is the default anyway; therefore it does not make
   sense to try to modify it from its default setting.

5. The __FAST_MATH__ preprocessor macro should be defined if and only
   if all the component flags of -ffast-math are set to the value that
   is documented as the effect of -ffast-math.  The following flags
   were currently *not* so tested:
     -fcx-limited-range
     -fassociative-math
     -freciprocal-math
     -frounding-math
   (Note that we should still *test* for -fno-rounding-math here even
   though it is not set as per 4.  -ffast-math -frounding-math should
   not set the __FAST_MATH__ macro.)
   This is also a documentation change.


Tested on s390x-ibm-linux.

OK for mainline?

Bye,
Ulrich

gcc/ChangeLog:

	* doc/invoke.texi (-ffast-math): Remove mention of -fno-signaling-nans.
	Clarify conditions when __FAST_MATH__ preprocessor macro is defined.

	* opts.c (common_handle_option): Pass OPTS_SET to set_fast_math_flags
	and set_unsafe_math_optimizations_flags.
	(set_fast_math_flags): Add OPTS_SET argument, and use it to avoid
	setting flags already explicitly set on the command line.  In the !set
	case, also reset x_flag_cx_limited_range and x_flag_excess_precision.
	Never reset x_flag_signaling_nans or x_flag_rounding_math.
	(set_unsafe_math_optimizations_flags): Add OPTS_SET argument, and use
	it to avoid setting flags already explicitly set on the command line.
	(fast_math_flags_set_p): Also test x_flag_cx_limited_range,
	x_flag_associative_math, x_flag_reciprocal_math, and
	x_flag_rounding_math.

Comments

Segher Boessenkool Feb. 11, 2020, 11:55 p.m. UTC | #1
Hi!

On Tue, Feb 11, 2020 at 07:43:31PM +0100, Ulrich Weigand wrote:
> 1. If a component flag of -ffast-math (or -funsafe-math-optimizations)
>    is explicitly set (or reset) on the command line, this should override
>    any implicit change due to -f(no-)fast-math, no matter in which order
>    the flags come on the command line.  This change affects all flags.

Ack.

> 2. Any component flag modified from its default by -ffast-math should
>    be reset to the default by -fno-fast-math.  This was previously
>    not done for the following flags:
>       -fcx-limited-range
>       -fexcess-precision=

Ack.

> 3. Once -ffinite-math-only is true, the -f(no-)signaling-nans flag has
>    no meaning (if we have no NaNs at all, it does not matter whether
>    there is a difference between quiet and signaling NaNs).  Therefore,
>    it does not make sense for -ffast-math to imply -fno-signaling-nans.
>    This is also a documentation change.

Ack.

> 4. -ffast-math is documented to imply -fno-rounding-math, however the
>    latter setting is the default anyway; therefore it does not make
>    sense to try to modify it from its default setting.

Ack.  It might be more robust to *do* set it, so that it still works if
the default ever changes.

> 5. The __FAST_MATH__ preprocessor macro should be defined if and only
>    if all the component flags of -ffast-math are set to the value that
>    is documented as the effect of -ffast-math.  The following flags
>    were currently *not* so tested:
>      -fcx-limited-range
>      -fassociative-math
>      -freciprocal-math
>      -frounding-math
>    (Note that we should still *test* for -fno-rounding-math here even
>    though it is not set as per 4.  -ffast-math -frounding-math should
>    not set the __FAST_MATH__ macro.)
>    This is also a documentation change.

Ack.

All looks good to me, but I'm not maintainer of this.

This should be mentioned in the release notes I think, it does change
behaviour (for the better, but any change is a change).

Thanks,


Segher
diff mbox series

Patch

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2cd8d7ec5ff..662fded21ea 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -11441,10 +11441,10 @@  is unpredictable.
 @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}, @option{-fcx-limited-range} and
-@option{-fexcess-precision=fast}.
+@option{-fcx-limited-range} and @option{-fexcess-precision=fast}.
 
-This option causes the preprocessor macro @code{__FAST_MATH__} to be defined.
+Whenever all these options listed above are set to those values,
+the preprocessor macro @code{__FAST_MATH__} will be defined.
 
 This option is not turned on by any @option{-O} option besides
 @option{-Ofast} since it can result in incorrect output for programs
diff --git a/gcc/opts.c b/gcc/opts.c
index 7affeb41a96..5a2ae9631ab 100644
--- a/gcc/opts.c
+++ b/gcc/opts.c
@@ -192,10 +192,12 @@  static void set_debug_level (enum debug_info_type type, int extended,
 			     const char *arg, struct gcc_options *opts,
 			     struct gcc_options *opts_set,
 			     location_t loc);
-static void set_fast_math_flags (struct gcc_options *opts, int set);
+static void set_fast_math_flags (struct gcc_options *opts,
+				 struct gcc_options *opts_set, int set);
 static void decode_d_option (const char *arg, struct gcc_options *opts,
 			     location_t loc, diagnostic_context *dc);
 static void set_unsafe_math_optimizations_flags (struct gcc_options *opts,
+						 struct gcc_options *opts_set,
 						 int set);
 static void enable_warning_as_error (const char *arg, int value,
 				     unsigned int lang_mask,
@@ -2447,11 +2449,11 @@  common_handle_option (struct gcc_options *opts,
       break;
 
     case OPT_ffast_math:
-      set_fast_math_flags (opts, value);
+      set_fast_math_flags (opts, opts_set, value);
       break;
 
     case OPT_funsafe_math_optimizations:
-      set_unsafe_math_optimizations_flags (opts, value);
+      set_unsafe_math_optimizations_flags (opts, opts_set, value);
       break;
 
     case OPT_ffixed_:
@@ -2839,44 +2841,44 @@  set_Wstrict_aliasing (struct gcc_options *opts, int onoff)
 /* The following routines are useful in setting all the flags that
    -ffast-math and -fno-fast-math imply.  */
 static void
-set_fast_math_flags (struct gcc_options *opts, int set)
+set_fast_math_flags (struct gcc_options *opts,
+		     struct gcc_options *opts_set, int set)
 {
-  if (!opts->frontend_set_flag_unsafe_math_optimizations)
+  if (!opts->frontend_set_flag_unsafe_math_optimizations
+      && !opts_set->x_flag_unsafe_math_optimizations)
     {
       opts->x_flag_unsafe_math_optimizations = set;
-      set_unsafe_math_optimizations_flags (opts, set);
+      set_unsafe_math_optimizations_flags (opts, opts_set, set);
     }
   if (!opts->frontend_set_flag_finite_math_only)
-    opts->x_flag_finite_math_only = set;
+    SET_OPTION_IF_UNSET (opts, opts_set, 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;
-    }
+    SET_OPTION_IF_UNSET (opts, opts_set, flag_errno_math, !set);
+  if (!opts->frontend_set_flag_cx_limited_range)
+    SET_OPTION_IF_UNSET (opts, opts_set, flag_cx_limited_range, set);
+  if (!opts->frontend_set_flag_excess_precision)
+    SET_OPTION_IF_UNSET (opts, opts_set, flag_excess_precision,
+			 set ? EXCESS_PRECISION_FAST
+			     : EXCESS_PRECISION_DEFAULT);
+
+  // -ffast-math should also reset -frounding-math, but since this
+  // is off by default, there's nothing to do for now.
 }
 
 /* When -funsafe-math-optimizations is set the following
    flags are set as well.  */
 static void
-set_unsafe_math_optimizations_flags (struct gcc_options *opts, int set)
+set_unsafe_math_optimizations_flags (struct gcc_options *opts,
+				     struct gcc_options *opts_set, int set)
 {
   if (!opts->frontend_set_flag_trapping_math)
-    opts->x_flag_trapping_math = !set;
+    SET_OPTION_IF_UNSET (opts, opts_set, flag_trapping_math, !set);
   if (!opts->frontend_set_flag_signed_zeros)
-    opts->x_flag_signed_zeros = !set;
+    SET_OPTION_IF_UNSET (opts, opts_set, flag_signed_zeros, !set);
   if (!opts->frontend_set_flag_associative_math)
-    opts->x_flag_associative_math = set;
+    SET_OPTION_IF_UNSET (opts, opts_set, flag_associative_math, set);
   if (!opts->frontend_set_flag_reciprocal_math)
-    opts->x_flag_reciprocal_math = set;
+    SET_OPTION_IF_UNSET (opts, opts_set, flag_reciprocal_math, set);
 }
 
 /* Return true iff flags in OPTS are set as if -ffast-math.  */
@@ -2884,10 +2886,14 @@  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_rounding_math
+	  && opts->x_flag_cx_limited_range
 	  && opts->x_flag_excess_precision == EXCESS_PRECISION_FAST);
 }