diff mbox

Patch RFA: Let languages control -ffast-math

Message ID mcrsjw2xru8.fsf@google.com
State New
Headers show

Commit Message

Ian Lance Taylor Feb. 5, 2011, 6:24 p.m. UTC 
As I pointed out in http://gcc.gnu.org/ml/gcc/2011-01/msg00454.html ,
now that the default options always include either -ffast-math or
-fno-fast-math, language specific settings for specific options
controlled by -ffast-math are overridden.  This affects at least
Fortran, Java, and Go.  In a reply Joseph suggested that -ffast-math
should not override language dependent defaults.  This patch implements
that suggestion.

This patch adds a new language hook apply_combined_option which lets the
frontend control whether a meta-option like -ffast-math should affect
another more specific option.  This is a general mechanism and is named
accordingly, but in fact at present it seems only appropriate for
-ffast-math and -funsafe-math-optimizations, so I would be willing to
give it a more specific name if that seems appropriate.

I did not change the documentation as this patch by itself does not
cause any change in behaviour.  If this patch is approved, subsequent
patches for the affected frontends would be appropriate.  For the Go
frontend the patch is to define the new language hook like this:

static bool
go_langhook_apply_combined_option (size_t combined ATTRIBUTE_UNUSED,
				   size_t specific)
{
  return specific != OPT_fmath_errno;
}

This patch creates the infrastructure for fixing a bug we have
introduced in 4.6 for Fortran and Java, so I think it is appropriate for
stage 3.

Bootstrapped and tested with all default languages except Java on
x86_64-unknown-linux-gnu.

OK for mainline?

Ian


2011-02-05  Ian Lance Taylor  <iant@google.com>

	* langhooks.h (struct lang_hooks): Add apply_combined_option.
	* langhooks-def.h (lhd_apply_combined_option): Declare.
	(LANG_HOOKS_APPLY_COMBINED_OPTION): Define.
	(LANG_HOOKS_INITIALIZER): Add LANG_HOOKS_APPLY_COMBINED_OPTION.
	* langhooks.c (lhd_apply_combined_option): New function.
	* opts.c: Include "tree.h" and "langhooks.h".
	(set_fast_math_flags): Check apply_combined_option before changing
	each field.
	(set_unsafe_math_optimizations_flags): Likewise.  Also add code
	parameter and change all callers.
	* Makefile.in (opts.o): Depend upon $(TREE_H) and langhooks.h.

Comments

Richard Biener Feb. 6, 2011, 11:07 a.m. UTC | #1 
On Sat, Feb 5, 2011 at 7:24 PM, Ian Lance Taylor <iant@google.com> wrote:
> As I pointed out in http://gcc.gnu.org/ml/gcc/2011-01/msg00454.html ,
> now that the default options always include either -ffast-math or
> -fno-fast-math, language specific settings for specific options
> controlled by -ffast-math are overridden.  This affects at least
> Fortran, Java, and Go.  In a reply Joseph suggested that -ffast-math
> should not override language dependent defaults.  This patch implements
> that suggestion.
>
> This patch adds a new language hook apply_combined_option which lets the
> frontend control whether a meta-option like -ffast-math should affect
> another more specific option.  This is a general mechanism and is named
> accordingly, but in fact at present it seems only appropriate for
> -ffast-math and -funsafe-math-optimizations, so I would be willing to
> give it a more specific name if that seems appropriate.
>
> I did not change the documentation as this patch by itself does not
> cause any change in behaviour.  If this patch is approved, subsequent
> patches for the affected frontends would be appropriate.  For the Go
> frontend the patch is to define the new language hook like this:
>
> static bool
> go_langhook_apply_combined_option (size_t combined ATTRIBUTE_UNUSED,
>                                   size_t specific)
> {
>  return specific != OPT_fmath_errno;
> }
>
> This patch creates the infrastructure for fixing a bug we have
> introduced in 4.6 for Fortran and Java, so I think it is appropriate for
> stage 3.
>
> Bootstrapped and tested with all default languages except Java on
> x86_64-unknown-linux-gnu.
>
> OK for mainline?

Can you try to add a testcase?  It seems it should be possible to
construct one for Fortran at least, based on the fact it allows
FP association by default.

Richard.

> Ian
>
>
> 2011-02-05  Ian Lance Taylor  <iant@google.com>
>
>        * langhooks.h (struct lang_hooks): Add apply_combined_option.
>        * langhooks-def.h (lhd_apply_combined_option): Declare.
>        (LANG_HOOKS_APPLY_COMBINED_OPTION): Define.
>        (LANG_HOOKS_INITIALIZER): Add LANG_HOOKS_APPLY_COMBINED_OPTION.
>        * langhooks.c (lhd_apply_combined_option): New function.
>        * opts.c: Include "tree.h" and "langhooks.h".
>        (set_fast_math_flags): Check apply_combined_option before changing
>        each field.
>        (set_unsafe_math_optimizations_flags): Likewise.  Also add code
>        parameter and change all callers.
>        * Makefile.in (opts.o): Depend upon $(TREE_H) and langhooks.h.
>
>
>
Joseph Myers Feb. 6, 2011, 7:02 p.m. UTC | #2 
On Sat, 5 Feb 2011, Ian Lance Taylor wrote:

> This patch adds a new language hook apply_combined_option which lets the
> frontend control whether a meta-option like -ffast-math should affect
> another more specific option.  This is a general mechanism and is named
> accordingly, but in fact at present it seems only appropriate for
> -ffast-math and -funsafe-math-optimizations, so I would be willing to
> give it a more specific name if that seems appropriate.

opts.c is not meant to use langhooks; I moved anything that would be 
problematic to share with the driver into opts-global.c so that opts.c can 
in future be shared with the driver.  So it would be better in that 
regard, though less general, for the init_options langhook to set 
something in the gcc_options struct indicating that a particular option 
should not be controlled by -ffast-math.

(Various targets have linker specs 
%{ffast-math|funsafe-math-optimizations:crtfastmath.o%s} and it would be 
good if in future they could be based on logical state of 
flag_unsafe_math_optimizations in the driver, so knowing these 
implications in the driver is potentially useful.  But certainly such 
driver processing can only operate with language-independent defaults 
since it may be linking a multi-language program.)
diff mbox

Patch

Index: langhooks.h
===================================================================
--- langhooks.h	(revision 169848)
+++ langhooks.h	(working copy)
@@ -299,6 +299,17 @@  struct lang_hooks
 			 location_t loc,
 			 const struct cl_option_handlers *handlers);
 
+  /* Return true if the option COMBINED should affect the option
+     SPECIFIC.  This is used for options like -ffast-math which
+     control several other more specific options like -ferrno-math.
+     If this hook returns false then using the combined option will
+     not affect the specific option.  Note that the specific option
+     may still be changed directly; this only affects the behaviour of
+     the combined option.  This is not used for optimization options
+     (-O1, -Ofast, etc.).  The default version of this hook returns
+     true.  */
+  bool (*apply_combined_option) (size_t combined, size_t specific);
+
   /* Called when all command line options have been parsed to allow
      further processing and initialization
 
Index: langhooks-def.h
===================================================================
--- langhooks-def.h	(revision 169848)
+++ langhooks-def.h	(working copy)
@@ -69,6 +69,7 @@  extern void lhd_init_options (unsigned i
 extern bool lhd_complain_wrong_lang_p (const struct cl_option *);
 extern bool lhd_handle_option (size_t, const char *, int, int, location_t,
 			       const struct cl_option_handlers *);
+extern bool lhd_apply_combined_option (size_t, size_t);
 extern tree lhd_callgraph_analyze_expr (tree *, int *);
 
 
@@ -91,6 +92,7 @@  extern void lhd_omp_firstprivatize_type_
 #define LANG_HOOKS_INITIALIZE_DIAGNOSTICS lhd_initialize_diagnostics
 #define LANG_HOOKS_COMPLAIN_WRONG_LANG_P lhd_complain_wrong_lang_p
 #define LANG_HOOKS_HANDLE_OPTION	lhd_handle_option
+#define LANG_HOOKS_APPLY_COMBINED_OPTION lhd_apply_combined_option
 #define LANG_HOOKS_POST_OPTIONS		lhd_post_options
 #define LANG_HOOKS_MISSING_NORETURN_OK_P hook_bool_tree_true
 #define LANG_HOOKS_GET_ALIAS_SET	lhd_get_alias_set
@@ -267,6 +269,7 @@  extern void lhd_end_section (void);
   LANG_HOOKS_INITIALIZE_DIAGNOSTICS, \
   LANG_HOOKS_COMPLAIN_WRONG_LANG_P, \
   LANG_HOOKS_HANDLE_OPTION, \
+  LANG_HOOKS_APPLY_COMBINED_OPTION, \
   LANG_HOOKS_POST_OPTIONS, \
   LANG_HOOKS_INIT, \
   LANG_HOOKS_FINISH, \
Index: langhooks.c
===================================================================
--- langhooks.c	(revision 169848)
+++ langhooks.c	(working copy)
@@ -355,6 +355,14 @@  lhd_handle_option (size_t code ATTRIBUTE
   return false;
 }
 
+/* By default, combined options always apply.  */
+bool
+lhd_apply_combined_option (size_t combined ATTRIBUTE_UNUSED,
+			   size_t specific ATTRIBUTE_UNUSED)
+{
+  return true;
+}
+
 /* The default function to print out name of current function that caused
    an error.  */
 void
Index: opts.c
===================================================================
--- opts.c	(revision 169848)
+++ opts.c	(working copy)
@@ -23,6 +23,8 @@  along with GCC; see the file COPYING3.  
 #include "system.h"
 #include "intl.h"
 #include "coretypes.h"
+#include "tree.h" /* Required by langhooks.h.  */
+#include "langhooks.h"
 #include "tm.h" /* Needed by rtl.h and used for STACK_CHECK_BUILTIN,
 		   STACK_CHECK_STATIC_BUILTIN, DEFAULT_GDB_EXTENSIONS,
 		   DWARF2_DEBUGGING_INFO and DBX_DEBUGGING_INFO.  */
@@ -208,6 +210,7 @@  static void set_fast_math_flags (struct 
 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,
+						 size_t code,
 						 int set);
 static void enable_warning_as_error (const char *arg, int value,
 				     unsigned int lang_mask,
@@ -1476,7 +1479,9 @@  common_handle_option (struct gcc_options
       break;
 
     case OPT_funsafe_math_optimizations:
-      set_unsafe_math_optimizations_flags (opts, value);
+      set_unsafe_math_optimizations_flags (opts,
+					   OPT_funsafe_math_optimizations,
+					   value);
       break;
 
     case OPT_ffixed_:
@@ -1757,27 +1762,46 @@  set_Wstrict_aliasing (struct gcc_options
 static void
 set_fast_math_flags (struct gcc_options *opts, int set)
 {
-  opts->x_flag_unsafe_math_optimizations = set;
-  set_unsafe_math_optimizations_flags (opts, set);
-  opts->x_flag_finite_math_only = set;
-  opts->x_flag_errno_math = !set;
+  if (lang_hooks.apply_combined_option (OPT_ffast_math,
+					OPT_funsafe_math_optimizations))
+    {
+      opts->x_flag_unsafe_math_optimizations = set;
+      set_unsafe_math_optimizations_flags (opts, OPT_ffast_math, set);
+    }
+  if (lang_hooks.apply_combined_option (OPT_ffast_math,
+					OPT_ffinite_math_only))
+    opts->x_flag_finite_math_only = set;
+  if (lang_hooks.apply_combined_option (OPT_ffast_math,
+					OPT_fmath_errno))
+    opts->x_flag_errno_math = !set;
   if (set)
     {
-      opts->x_flag_signaling_nans = 0;
-      opts->x_flag_rounding_math = 0;
-      opts->x_flag_cx_limited_range = 1;
+      if (lang_hooks.apply_combined_option (OPT_ffast_math,
+					    OPT_fsignaling_nans))
+	opts->x_flag_signaling_nans = 0;
+      if (lang_hooks.apply_combined_option (OPT_ffast_math,
+					    OPT_frounding_math))
+	opts->x_flag_rounding_math = 0;
+      if (lang_hooks.apply_combined_option (OPT_ffast_math,
+					    OPT_fcx_limited_range))
+	opts->x_flag_cx_limited_range = 1;
     }
 }
 
-/* When -funsafe-math-optimizations is set the following
-   flags are set as well.  */
+/* When -funsafe-math-optimizations is set the following flags are set
+   as well.  CODE is the option code causing this to be called.  */
 static void
-set_unsafe_math_optimizations_flags (struct gcc_options *opts, int set)
+set_unsafe_math_optimizations_flags (struct gcc_options *opts, size_t code,
+				     int set)
 {
-  opts->x_flag_trapping_math = !set;
-  opts->x_flag_signed_zeros = !set;
-  opts->x_flag_associative_math = set;
-  opts->x_flag_reciprocal_math = set;
+  if (lang_hooks.apply_combined_option (code, OPT_ftrapping_math))
+    opts->x_flag_trapping_math = !set;
+  if (lang_hooks.apply_combined_option (code, OPT_fsigned_zeros))
+    opts->x_flag_signed_zeros = !set;
+  if (lang_hooks.apply_combined_option (code, OPT_fassociative_math))
+    opts->x_flag_associative_math = set;
+  if (lang_hooks.apply_combined_option (code, OPT_freciprocal_math))
+    opts->x_flag_reciprocal_math = set;
 }
 
 /* Return true iff flags in OPTS are set as if -ffast-math.  */
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 169848)
+++ Makefile.in	(working copy)
@@ -2780,7 +2780,7 @@  fold-const.o : fold-const.c $(CONFIG_H) 
 diagnostic.o : diagnostic.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
    version.h $(INPUT_H) intl.h $(DIAGNOSTIC_H) diagnostic.def
 opts.o : opts.c $(OPTS_H) $(OPTIONS_H) $(DIAGNOSTIC_CORE_H) $(CONFIG_H) $(SYSTEM_H) \
-   coretypes.h $(TM_H) $(RTL_H) \
+   coretypes.h $(TREE_H) langhooks.h $(TM_H) $(RTL_H) \
    $(DIAGNOSTIC_H) $(INSN_ATTR_H) intl.h $(TARGET_H) \
    $(FLAGS_H) $(PARAMS_H) opts-diagnostic.h
 opts-global.o : opts-global.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \