diff mbox

[6/11] Migrate excess precision logic to use TARGET_EXCESS_PRECISION

Message ID 1475254617-10825-4-git-send-email-james.greenhalgh@arm.com
State New
Headers show

Commit Message

James Greenhalgh Sept. 30, 2016, 4:56 p.m. UTC 
Hi,

This patch moves the logic for excess precision from using the
TARGET_FLT_EVAL_METHOD macro to the TARGET_EXCESS_PRECISION hook
introduced earlier in the patch series.

These logic changes follow Joseph's comments at
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00410.html

Briefly; we have four things to change.

  1) The logic in tree.c::excess_precision_type .
  Here we want to ask the target which excess preicion it would like for
  whichever of -fexcess-precision=standard or -fexcess-precision=fast is
  in use, then apply that.

  2) The logic in c-family/c-cppbuiltin.c::c_cpp_flt_eval_method_iec_559 .
  We want to update this to ensure that the target claims the same excess
  precision to be implicitly added to operations that it reports in
  -fexcess-precision=standard mode. We take the join of these two reported
  values, and only if the join is equal to the excess precision requested
  for -fexcess-precision=standard can we set the IEC_559 macro.

  3) The logic in c-family/c-cppbuiltin.c::c_cpp_builtin for setting
  __FLT_EVAL_METHOD__ .
  Which is now little more complicated, and makes use of
  -fpermitted-flt-eval-methods from patch 5.

  4) The logic in c-family/c-cppbuiltin.c::c_cpp_builtin for setting
  __LIBGCC_*_EXCESS_PRECISION__ .
  This can just be the implicit precision reported by the target.

Having moved the logic in to those areas, we can simplify
toplev.c::init_excess_precision , which now only retains the assert that
-fexcess-precision=default has been rewritten by the language front-end, and
the set from the command-line variable to the internal variable.

The documentation in invoke.texi is not quite right for the impact of
-fexcess-precision, so I've rewritten the text to read a little more
generic.

Bootstrapped on x86_64 and aarch64 with no issues.

Thanks,
James

---
gcc/

2016-09-30  James Greenhalgh  <james.greenhalgh@arm.com>

	* toplev.c (init_excess_precision): Delete most logic.
	* tree.c (excess_precision_type): Rewrite to use
	TARGET_EXCESS_PRECISION.
	* doc/invoke.texi (-fexcess-precision): Document behaviour in a
	more generic fashion.

gcc/c-family/

2016-09-30  James Greenhalgh  <james.greenhalgh@arm.com>

	* c-common.c (excess_precision_mode_join): New.
	(c_ts18661_flt_eval_method): New.
	(c_c11_flt_eval_method): Likewise.
	(c_flt_eval_method): Likewise.
	* c-common.h (excess_precision_mode_join): New.
	(c_flt_eval_method): Likewise.
	* c-cppbuiltin.c (c_cpp_flt_eval_method_iec_559): New.
	(cpp_iec_559_value): Call it.
	(c_cpp_builtins): Modify logic for __LIBGCC_*_EXCESS_PRECISION__,
	call c_flt_eval_method to set __FLT_EVAL_METHOD__ and
	__FLT_EVAL_METHOD_C99__.

Comments

Joseph Myers Sept. 30, 2016, 5:32 p.m. UTC | #1 
On Fri, 30 Sep 2016, James Greenhalgh wrote:

>    /* float.h needs to know this.  */
> +  /* We already have the option -fno-fp-int-builtin-inexact to ensure
> +     certain built-in functions follow TS 18661-1 semantics.  It might be
> +     reasonable to have a new option to enable FLT_EVAL_METHOD using new
> +     values.  However, I'd be inclined to think that such an option should
> +     be on by default for -std=gnu*, only off for strict conformance modes.
> +     (There would be both __FLT_EVAL_METHOD__ and __FLT_EVAL_METHOD_C99__,
> +     say, predefined macros, so that <float.h> could also always use the
> +     new value if __STDC_WANT_IEC_60559_TYPES_EXT__ is defined.)  */

This comment makes no sense in the context.  The comment should not be 
talking about some other option for a different issue, or about 
half-thought-out ideas for how something might be implemented; comments 
need to relate to the actual code (which in this case is obvious and not 
in need of comments beyond saying what the macro semantics are).  In any 
case, this patch does not achieve the proposed semantics, since there is 
no change to ginclude/float.h.

The goal is: if the user's options imply new FLT_EVAL_METHOD values are 
OK, *or* they defined __STDC_WANT_IEC_60559_TYPES_EXT__ before including 
<float.h>, it should use the appropriate TS 18661-3 value.  Otherwise 
(strict standards modes for existing standards, no 
__STDC_WANT_IEC_60559_TYPES_EXT__) it should use a C11 value.

So in a strict standards mode you need to predefine macros with both 
choices of values and let <float.h> choose between them.  One possibility 
is: __FLT_EVAL_METHOD_C99__ is the value to use when 
__STDC_WANT_IEC_60559_TYPES_EXT__ is not defined, __FLT_EVAL_METHOD__ is 
the value to use when it is defined.  Or some other arrangement, with or 
without a macro saying what setting you have for the new option.  But you 
can't avoid changing <float.h>.

Tests then should be testing the value of FLT_EVAL_METHOD from <float.h>, 
*not* the internal macros predefined by the compiler.
diff mbox

Patch

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 2652259..983f71a 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -13145,4 +13145,83 @@  diagnose_mismatched_attributes (tree olddecl, tree newdecl)
   return warned;
 }
 
+/* Return the latice point which is the wider of the two FLT_EVAL_METHOD
+   modes X, Y.  This isn't just  >, as the FLT_EVAL_METHOD values added
+   by C TS 18661-3 for interchange  types that are computed in their
+   native precision are larger than the C11 values for evaluating in the
+   precision of float/double/long double.  If either mode is
+   FLT_EVAL_METHOD_UNPREDICTABLE, return that.  */
+
+enum flt_eval_method
+excess_precision_mode_join (enum flt_eval_method x,
+			    enum flt_eval_method y)
+{
+  if (x == FLT_EVAL_METHOD_UNPREDICTABLE
+      || y == FLT_EVAL_METHOD_UNPREDICTABLE)
+    return FLT_EVAL_METHOD_UNPREDICTABLE;
+
+  /* GCC only supports one interchange type right now, _Float16.  If
+     we're evaluating _Float16 in 16-bit precision, then flt_eval_method
+     will be FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16.  */
+  if (x == FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16)
+    return y;
+  if (y == FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16)
+    return x;
+
+  /* Other values for flt_eval_method are directly comparable, and we want
+     the maximum.  */
+  return MAX (x, y);
+}
+
+/* Return the value that should be set for FLT_EVAL_METHOD in the
+   context of ISO/IEC TS 18861-3.
+
+   This should relate to the effective excess precision seen by the user,
+   which is the join point of the precision the target requests for
+   -fexcess-precision={standard,fast} and the implicit excess precision
+   the target uses.  */
+
+static enum flt_eval_method
+c_ts18661_flt_eval_method (void)
+{
+  enum flt_eval_method implicit
+    = targetm.c.excess_precision (EXCESS_PRECISION_TYPE_IMPLICIT);
+
+  enum excess_precision_type flag_type
+    = (flag_excess_precision_cmdline == EXCESS_PRECISION_STANDARD
+       ? EXCESS_PRECISION_TYPE_STANDARD
+       : EXCESS_PRECISION_TYPE_FAST);
+
+  enum flt_eval_method requested
+    = targetm.c.excess_precision (flag_type);
+
+  return excess_precision_mode_join (implicit, requested);
+}
+
+/* As c_cpp_ts18661_flt_eval_method, but clamps the expected values to
+   those that were permitted by C11.  That is to say, eliminates
+   FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16.  */
+
+static enum flt_eval_method
+c_c11_flt_eval_method (void)
+{
+  return excess_precision_mode_join (c_ts18661_flt_eval_method (),
+				     FLT_EVAL_METHOD_PROMOTE_TO_FLOAT);
+}
+
+/* Return the value that should be set for FLT_EVAL_METHOD.  TS18661_P
+   is TRUE if we are in a context where values from ISO/IEEC TS 18861-3
+   are permitted, and FALSE otherwise.  See the comments on
+   c_ts18661_flt_eval_method for what value we choose to set here.  */
+
+int
+c_flt_eval_method (bool ts18661_p)
+{
+  if (ts18661_p && flag_permitted_flt_eval_methods
+		== PERMITTED_FLT_EVAL_METHODS_TS_18661)
+    return c_ts18661_flt_eval_method ();
+  else
+    return c_c11_flt_eval_method ();
+}
+
 #include "gt-c-family-c-common.h"
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index c88619b..0c94207 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1532,6 +1532,11 @@  extern bool valid_array_size_p (location_t, tree, tree);
 extern bool cilk_ignorable_spawn_rhs_op (tree);
 extern bool cilk_recognize_spawn (tree, tree *);
 
+extern enum flt_eval_method
+excess_precision_mode_join (enum flt_eval_method, enum flt_eval_method);
+
+extern int c_flt_eval_method (bool ts18661_p);
+
 #if CHECKING_P
 namespace selftest {
   extern void c_format_c_tests (void);
diff --git a/gcc/c-family/c-cppbuiltin.c b/gcc/c-family/c-cppbuiltin.c
index b860c21..c8d5290 100644
--- a/gcc/c-family/c-cppbuiltin.c
+++ b/gcc/c-family/c-cppbuiltin.c
@@ -727,6 +727,32 @@  cpp_atomic_builtins (cpp_reader *pfile)
 			(have_swap[psize]? 2 : 1));
 }
 
+/* If the join of the implicit precision in which the target will compute
+   floating-point values and the standard precision in which the target will
+   compute values is not equal to the standard precision, then the target
+   is either unpredictable, or is a broken configuration in which it claims
+   standards compliance, but doesn't honor that.
+
+   Effective predictability for __GCC_IEC_559 in flag_iso_mode, means that
+   the implicit precision is not wider, or less predictable than the
+   standard precision.
+
+   Return TRUE if we have been asked to compile with
+   -fexcess-precision=standard, and following the rules above we are able
+   to guarantee the standards mode.  */
+
+static bool
+c_cpp_flt_eval_method_iec_559 (void)
+{
+  enum flt_eval_method implicit
+    = targetm.c.excess_precision (EXCESS_PRECISION_TYPE_IMPLICIT);
+  enum flt_eval_method standard
+    = targetm.c.excess_precision (EXCESS_PRECISION_TYPE_STANDARD);
+
+  return (excess_precision_mode_join (implicit, standard) == standard
+	  && flag_excess_precision_cmdline == EXCESS_PRECISION_STANDARD);
+}
+
 /* Return the value for __GCC_IEC_559.  */
 static int
 cpp_iec_559_value (void)
@@ -774,11 +800,12 @@  cpp_iec_559_value (void)
      applies to unpredictable contraction.  For C++, and outside
      strict conformance mode, do not consider these options to mean
      lack of IEEE 754 support.  */
+
   if (flag_iso
       && !c_dialect_cxx ()
-      && TARGET_FLT_EVAL_METHOD != 0
-      && flag_excess_precision_cmdline != EXCESS_PRECISION_STANDARD)
+      && !c_cpp_flt_eval_method_iec_559 ())
     ret = 0;
+
   if (flag_iso
       && !c_dialect_cxx ()
       && flag_fp_contract_mode == FP_CONTRACT_FAST)
@@ -1038,8 +1065,18 @@  c_cpp_builtins (cpp_reader *pfile)
 				 cpp_iec_559_complex_value ());
 
   /* float.h needs to know this.  */
+  /* We already have the option -fno-fp-int-builtin-inexact to ensure
+     certain built-in functions follow TS 18661-1 semantics.  It might be
+     reasonable to have a new option to enable FLT_EVAL_METHOD using new
+     values.  However, I'd be inclined to think that such an option should
+     be on by default for -std=gnu*, only off for strict conformance modes.
+     (There would be both __FLT_EVAL_METHOD__ and __FLT_EVAL_METHOD_C99__,
+     say, predefined macros, so that <float.h> could also always use the
+     new value if __STDC_WANT_IEC_60559_TYPES_EXT__ is defined.)  */
   builtin_define_with_int_value ("__FLT_EVAL_METHOD__",
-				 TARGET_FLT_EVAL_METHOD);
+				 c_flt_eval_method (true));
+  builtin_define_with_int_value ("__FLT_EVAL_METHOD_C99__",
+				 c_flt_eval_method (false));
 
   /* And decfloat.h needs this.  */
   builtin_define_with_int_value ("__DEC_EVAL_METHOD__",
@@ -1180,25 +1217,38 @@  c_cpp_builtins (cpp_reader *pfile)
 	      gcc_assert (found_suffix);
 	    }
 	  builtin_define_with_value (macro_name, suffix, 0);
+
+	  /* The way __LIBGCC_*_EXCESS_PRECISION__ is used is about
+	     eliminating excess precision from results assigned to
+	     variables - meaning it should be about the implicit excess
+	     precision only.  */
 	  bool excess_precision = false;
-	  if (TARGET_FLT_EVAL_METHOD != 0
-	      && mode != TYPE_MODE (long_double_type_node)
-	      && (mode == TYPE_MODE (float_type_node)
-		  || mode == TYPE_MODE (double_type_node)))
-	    switch (TARGET_FLT_EVAL_METHOD)
-	      {
-	      case -1:
-	      case 2:
-		excess_precision = true;
-		break;
-
-	      case 1:
-		excess_precision = mode == TYPE_MODE (float_type_node);
-		break;
-
-	      default:
-		gcc_unreachable ();
-	      }
+	  machine_mode float16_type_mode = (float16_type_node
+					    ? TYPE_MODE (float16_type_node)
+					    : VOIDmode);
+	  switch (targetm.c.excess_precision
+		    (EXCESS_PRECISION_TYPE_IMPLICIT))
+	    {
+	    case FLT_EVAL_METHOD_UNPREDICTABLE:
+	    case FLT_EVAL_METHOD_PROMOTE_TO_LONG_DOUBLE:
+	      excess_precision = (mode == float16_type_mode
+				  || mode == TYPE_MODE (float_type_node)
+				  || mode == TYPE_MODE (double_type_node));
+	      break;
+
+	    case FLT_EVAL_METHOD_PROMOTE_TO_DOUBLE:
+	      excess_precision = (mode == float16_type_mode
+				  || mode == TYPE_MODE (float_type_node));
+	      break;
+	    case FLT_EVAL_METHOD_PROMOTE_TO_FLOAT:
+	      excess_precision = mode == float16_type_mode;
+	      break;
+	    case FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16:
+	      excess_precision = false;
+	      break;
+	    default:
+	      gcc_unreachable ();
+	    }
 	  macro_name = (char *) alloca (strlen (name)
 					+ sizeof ("__LIBGCC__EXCESS_"
 						  "PRECISION__"));
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9cb0b54..ba6dc93 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8723,15 +8723,14 @@  them to store all pertinent intermediate computations into variables.
 @item -fexcess-precision=@var{style}
 @opindex fexcess-precision
 This option allows further control over excess precision on machines
-where floating-point registers have more precision than the IEEE
-@code{float} and @code{double} types and the processor does not
-support operations rounding to those types.  By default,
-@option{-fexcess-precision=fast} is in effect; this means that
-operations are carried out in the precision of the registers and that
-it is unpredictable when rounding to the types specified in the source
-code takes place.  When compiling C, if
-@option{-fexcess-precision=standard} is specified then excess
-precision follows the rules specified in ISO C99; in particular,
+where floating-point operations occur in a format with more precision or
+range than the IEEE standard and interchange floating-point types.  By
+default, @option{-fexcess-precision=fast} is in effect; this means that
+operations may be carried out in a wider precision than the types specified
+in the source if that would result in faster code, and it is unpredictable
+when rounding to the types specified in the source code takes place.
+When compiling C, if @option{-fexcess-precision=standard} is specified then
+excess precision follows the rules specified in ISO C99; in particular,
 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
diff --git a/gcc/toplev.c b/gcc/toplev.c
index 5f80763..9b3abab 100644
--- a/gcc/toplev.c
+++ b/gcc/toplev.c
@@ -1684,41 +1684,17 @@  backend_init (void)
   init_regs ();
 }
 
-/* Initialize excess precision settings.  */
+/* Initialize excess precision settings.
+
+   We have no need to modify anything here, just keep track of what the
+   user requested.  We'll figure out any appropriate relaxations
+   later.  */
+
 static void
 init_excess_precision (void)
 {
-  /* Adjust excess precision handling based on the target options.  If
-     the front end cannot handle it, flag_excess_precision_cmdline
-     will already have been set accordingly in the post_options
-     hook.  */
   gcc_assert (flag_excess_precision_cmdline != EXCESS_PRECISION_DEFAULT);
   flag_excess_precision = flag_excess_precision_cmdline;
-  if (flag_unsafe_math_optimizations)
-    flag_excess_precision = EXCESS_PRECISION_FAST;
-  if (flag_excess_precision == EXCESS_PRECISION_STANDARD)
-    {
-      int flt_eval_method = TARGET_FLT_EVAL_METHOD;
-      switch (flt_eval_method)
-	{
-	case -1:
-	case 0:
-	  /* Either the target acts unpredictably (-1) or has all the
-	     operations required not to have excess precision (0).  */
-	  flag_excess_precision = EXCESS_PRECISION_FAST;
-	  break;
-	case 1:
-	case 2:
-	  /* In these cases, predictable excess precision makes
-	     sense.  */
-	  break;
-	default:
-	  /* Any other implementation-defined FLT_EVAL_METHOD values
-	     require the compiler to handle the associated excess
-	     precision rules in excess_precision_type.  */
-	  gcc_unreachable ();
-	}
-    }
 }
 
 /* Initialize things that are both lang-dependent and target-dependent.
diff --git a/gcc/tree.c b/gcc/tree.c
index 33e6f97..678e244 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -8836,50 +8836,94 @@  build_complex_type (tree component_type)
 tree
 excess_precision_type (tree type)
 {
-  if (flag_excess_precision != EXCESS_PRECISION_FAST)
+  /* The target can give two different responses to the question of
+     which excess precision mode it would like depending on whether we
+     are in -fexcess-precision=standard or -fexcess-precision=fast.  */
+
+  enum excess_precision_type requested_type
+    = (flag_excess_precision == EXCESS_PRECISION_FAST
+       ? EXCESS_PRECISION_TYPE_FAST
+       : EXCESS_PRECISION_TYPE_STANDARD);
+
+  enum flt_eval_method target_flt_eval_method
+    = targetm.c.excess_precision (requested_type);
+
+  /* The target should not ask for unpredictable float evaluation (though
+     it might advertise that implicitly the evaluation is unpredictable,
+     but we don't care about that here, it will have been reported
+     elsewhere).  If it does ask for unpredictable evaluation, we have
+     nothing to do here.  */
+  gcc_assert (target_flt_eval_method != FLT_EVAL_METHOD_UNPREDICTABLE);
+
+  /* Nothing to do.  The target has asked for all types we know about
+     to be computed with their native precision and range.  */
+  if (target_flt_eval_method == FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16)
+    return NULL_TREE;
+
+  machine_mode float16_type_mode = (float16_type_node
+				    ? TYPE_MODE (float16_type_node)
+				    : VOIDmode);
+  machine_mode float_type_mode = TYPE_MODE (float_type_node);
+  machine_mode double_type_mode = TYPE_MODE (double_type_node);
+
+  switch (TREE_CODE (type))
     {
-      int flt_eval_method = TARGET_FLT_EVAL_METHOD;
-      switch (TREE_CODE (type))
-	{
-	case REAL_TYPE:
-	  switch (flt_eval_method)
-	    {
-	    case 1:
-	      if (TYPE_MODE (type) == TYPE_MODE (float_type_node))
-		return double_type_node;
-	      break;
-	    case 2:
-	      if (TYPE_MODE (type) == TYPE_MODE (float_type_node)
-		  || TYPE_MODE (type) == TYPE_MODE (double_type_node))
-		return long_double_type_node;
-	      break;
-	    default:
-	      gcc_unreachable ();
-	    }
-	  break;
-	case COMPLEX_TYPE:
-	  if (TREE_CODE (TREE_TYPE (type)) != REAL_TYPE)
-	    return NULL_TREE;
-	  switch (flt_eval_method)
-	    {
-	    case 1:
-	      if (TYPE_MODE (TREE_TYPE (type)) == TYPE_MODE (float_type_node))
-		return complex_double_type_node;
-	      break;
-	    case 2:
-	      if (TYPE_MODE (TREE_TYPE (type)) == TYPE_MODE (float_type_node)
-		  || (TYPE_MODE (TREE_TYPE (type))
-		      == TYPE_MODE (double_type_node)))
-		return complex_long_double_type_node;
-	      break;
-	    default:
-	      gcc_unreachable ();
-	    }
-	  break;
-	default:
-	  break;
-	}
+    case REAL_TYPE:
+      {
+	machine_mode type_mode = TYPE_MODE (type);
+	switch (target_flt_eval_method)
+	  {
+	  case FLT_EVAL_METHOD_PROMOTE_TO_FLOAT:
+	    if (type_mode == float16_type_mode)
+	      return float_type_node;
+	    break;
+	  case FLT_EVAL_METHOD_PROMOTE_TO_DOUBLE:
+	    if (type_mode == float16_type_mode
+		|| type_mode == float_type_mode)
+	      return double_type_node;
+	    break;
+	  case FLT_EVAL_METHOD_PROMOTE_TO_LONG_DOUBLE:
+	    if (type_mode == float16_type_mode
+		|| type_mode == float_type_mode
+		|| type_mode == double_type_mode)
+	      return long_double_type_node;
+	    break;
+	  default:
+	    gcc_unreachable ();
+	  }
+	break;
+      }
+    case COMPLEX_TYPE:
+      {
+	if (TREE_CODE (TREE_TYPE (type)) != REAL_TYPE)
+	  return NULL_TREE;
+	machine_mode type_mode = TYPE_MODE (TREE_TYPE (type));
+	switch (target_flt_eval_method)
+	  {
+	  case FLT_EVAL_METHOD_PROMOTE_TO_FLOAT:
+	    if (type_mode == float16_type_mode)
+	      return complex_float_type_node;
+	    break;
+	  case FLT_EVAL_METHOD_PROMOTE_TO_DOUBLE:
+	    if (type_mode == float16_type_mode
+		|| type_mode == float_type_mode)
+	      return complex_double_type_node;
+	    break;
+	  case FLT_EVAL_METHOD_PROMOTE_TO_LONG_DOUBLE:
+	    if (type_mode == float16_type_mode
+		|| type_mode == float_type_mode
+		|| type_mode == double_type_mode)
+	      return complex_long_double_type_node;
+	    break;
+	  default:
+	    gcc_unreachable ();
+	  }
+	break;
+      }
+    default:
+      break;
     }
+
   return NULL_TREE;
 }