Fix -ffast-math flags handling inconsistencies

Hello,

we've noticed some inconsistencies in how the component flags of -ffast-math
are handled, see the discussion on the GCC list starting here:
https://gcc.gnu.org/ml/gcc/2020-01/msg00365.html

This patch fixes those inconsistencies.  Specifically, there are the
following changes:

1. Some component flags for -ffast-math are *set* with -ffast-math
   (changing the default), but are not reset with -fno-fast-math,
   causing the latter to have surprising results.  (Note that since
   "-ffast-math -fno-fast-math" is short-cut by the driver, you'll
   only see the surprising results with "-Ofast -fno-fast-math".)
   This is fixed here by both setting and resetting the flags.

   This affects the following flags
      -fcx-limited-range
      -fexcess-precision=

2. Some component flags for -ffast-math are changed from their default,
   but are *not* included in the fast_math_flags_set_p test, causing
   __FAST_MATH__ to remain predefined even when the full set of fast
   math options is not actually in effect.  This is fixed here by
   adding those flags into the fast_math_flags_set_p test.

   This affects the following flags:
     -fcx-limited-range
     -fassociative-math
     -freciprocal-math

3. For some math flags, set_fast_math_flags has code that sets their
   values only to what is already their default.  The overall effect
   of this code is a complete no-op.  This patch removes that dead code.

   This affects the following flags:
     -frounding-math
     -fsignaling-nans


The overall effect of this patch is that now all component flags of
-ffast-math are treated exactly equivalently:
  * they are set (changed from their default) with -ffast-math
  * they are reset to their default with -fno-fast-math
  * __FAST_MATH__ is only defined if the value of the flag matches
    what -ffast-math would have set it to

Tested on s390x-ibm-linux.

OK for mainline?

Bye,
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.

On Fri, Jan 31, 2020 at 6:01 PM Ulrich Weigand <uweigand@de.ibm.com> wrote:
>
> Hello,
>
> we've noticed some inconsistencies in how the component flags of -ffast-math
> are handled, see the discussion on the GCC list starting here:
> https://gcc.gnu.org/ml/gcc/2020-01/msg00365.html
>
> This patch fixes those inconsistencies.  Specifically, there are the
> following changes:
>
> 1. Some component flags for -ffast-math are *set* with -ffast-math
>    (changing the default), but are not reset with -fno-fast-math,
>    causing the latter to have surprising results.  (Note that since
>    "-ffast-math -fno-fast-math" is short-cut by the driver, you'll
>    only see the surprising results with "-Ofast -fno-fast-math".)
>    This is fixed here by both setting and resetting the flags.
>
>    This affects the following flags
>       -fcx-limited-range
>       -fexcess-precision=
>
> 2. Some component flags for -ffast-math are changed from their default,
>    but are *not* included in the fast_math_flags_set_p test, causing
>    __FAST_MATH__ to remain predefined even when the full set of fast
>    math options is not actually in effect.  This is fixed here by
>    adding those flags into the fast_math_flags_set_p test.
>
>    This affects the following flags:
>      -fcx-limited-range
>      -fassociative-math
>      -freciprocal-math
>
> 3. For some math flags, set_fast_math_flags has code that sets their
>    values only to what is already their default.  The overall effect
>    of this code is a complete no-op.  This patch removes that dead code.
>
>    This affects the following flags:
>      -frounding-math
>      -fsignaling-nans
>
>
> The overall effect of this patch is that now all component flags of
> -ffast-math are treated exactly equivalently:
>   * they are set (changed from their default) with -ffast-math
>   * they are reset to their default with -fno-fast-math
>   * __FAST_MATH__ is only defined if the value of the flag matches
>     what -ffast-math would have set it to

The last part is not obviously correct to me since it doesn't match
documentation which says

@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}, @option{-fcx-limited-range} and
@option{-fexcess-precision=fast}.

This option causes the preprocessor macro @code{__FAST_MATH__} to be defined.

to me this reads as -ffast-math -fexcess-precision=standard defines
__FAST_MATH__.
The only relevant part to define __FAST_MATH__ is specifying -ffast-math, other
options are not relevant (which of course is contradicted by
implementation - where
I didn't actually follow its history in that respect).  So can you
adjust documentation
as to when exactly __FAST_MATH__ is defined?

Also...

> Tested on s390x-ibm-linux.
>
> OK for mainline?
>
> Bye,
> 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.
...

but what happens to -fsignalling-nans -ffast-math then?  Better leave those
in I'd say.

Note frontends come into play with what is considered -ffast-math
and -fno-fast-math but below flags are tested irrespectively of that
interpretation.

Note there's -fcx-fortran-rules similar to -fcx-limited-range but not tested
above.  The canonical middle-end "flag" to look at is flag_complex_method.
Somehow -fcx-fortran-rules doesn't come into play at all above but it
affects -fcx-limited-range in another inconsistent way in that
-fcx-limited-range -fcx-fortran-rules and -fcx-fortran-rules -fcx-limited-range
behave the same (-fcx-fortran-rules takes precedence...).  I guess
-fcomplex-method=ENUM should be exposed and -fcx-* made
appropriate aliases here.

You're tapping into a mine-field ;)

Richard.

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

On Fri, Feb 07, 2020 at 05:47:32PM +0100, Ulrich Weigand wrote:
> > but what happens to -fsignalling-nans -ffast-math then?  Better leave those
> > in I'd say.
> 
> Ah, it seems I was confused about the intended semantics here.
> 
> I thought that a *more specific* option like -fsignalling-nans was always
> intended to override a more generic option like -ffast-math, no matter
> whether it comes before or after it on the command line.

-fsignaling-nans is an independent option.  Signaling NaNs don't do much
at all when you also have -ffinite-math-only, of course, which says you
don't have *any* NaNs.


Segher