Patchwork [PR,debug/47590] rework md option overriding to delay var-tracking

login
register
mail settings
Submitter Alexandre Oliva
Date April 2, 2011, 8:15 a.m.
Message ID <orsju1ysq4.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/89432/
State New
Headers show

Comments

Alexandre Oliva - April 2, 2011, 8:15 a.m.
Some targets delayed the var-tracking pass to run it after
machine-specific transformations.  The introduction of option saving and
restoring broke this, because the machine-specific overriding took place
too late for it to be saved, so, after compiling a function that used a
different set of options, we'd restore incorrect flags, running
var-tracking at the wrong time.

This patch fixes the handling of this option so that it takes place at
the right time.  It does not, however, support per-function overriding
of -fvar-tracking; I'm not sure how to implement that with the current
framework.  Suggestions?

Meanwhile, is this ok to install?  I believe it should be applied to
trunk and 4.6 as well.  Although I've only tested it myself on trunk,
and on platforms that were not affected, a comment in the bug report
indicates it was tested (on trunk?) on one of the affected platforms.
Bernd Schmidt - May 4, 2011, 6:41 p.m.
On 04/02/2011 10:15 AM, Alexandre Oliva wrote:
> Some targets delayed the var-tracking pass to run it after
> machine-specific transformations.  The introduction of option saving and
> restoring broke this, because the machine-specific overriding took place
> too late for it to be saved, so, after compiling a function that used a
> different set of options, we'd restore incorrect flags, running
> var-tracking at the wrong time.
> 
> This patch fixes the handling of this option so that it takes place at
> the right time.  It does not, however, support per-function overriding
> of -fvar-tracking; I'm not sure how to implement that with the current
> framework.  Suggestions?

> @@ -5722,6 +5715,13 @@ ia64_option_override (void)
>    if (TARGET_ABI_OPEN_VMS)
>      flag_no_common = 1;
>  
> +  /* Variable tracking should be run after all optimizations which change order
> +     of insns.  It also needs a valid CFG.  This can't be done in
> +     ia64_option_override, because flag_var_tracking is finalized after
> +     that.  */

This comment looks very weird when added to ia64_option_override
(likewise for other targets). Is there a reason it's not true anymore?


Bernd

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/47590
	* config/bfin/bfin.c (output_file_start): Move flag_var_tracking
	overriding...
	(bfin_option_override): ... here.
	* config/ia64/ia64.c (ia64_file_start): Likewise...
	(ia64_option_override): ... ditto.
	* config/spu/spu.c (asm_file_start): Likewise...
	(spu_option_override): ... ditto.
	* config/picochip/picochip.c (picochip_asm_file_start): Likewise...
	(picochip_option_override): ... ditto.  Split previous code into...
	(picochip_override_options_after_change): ... this new function.
	(TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE): Use the latter.

Index: gcc/config/bfin/bfin.c
===================================================================
--- gcc/config/bfin/bfin.c.orig	2011-03-27 09:15:56.000000000 -0300
+++ gcc/config/bfin/bfin.c	2011-03-31 03:58:54.968115465 -0300
@@ -343,13 +343,6 @@  output_file_start (void) 
   FILE *file = asm_out_file;
   int i;
 
-  /* Variable tracking should be run after all optimizations which change order
-     of insns.  It also needs a valid CFG.  This can't be done in
-     bfin_option_override, because flag_var_tracking is finalized after
-     that.  */
-  bfin_flag_var_tracking = flag_var_tracking;
-  flag_var_tracking = 0;
-
   fprintf (file, ".file \"%s\";\n", input_filename);
   
   for (i = 0; arg_regs[i] >= 0; i++)
@@ -2723,6 +2716,13 @@  bfin_option_override (void)
   bfin_flag_schedule_insns2 = flag_schedule_insns_after_reload;
   flag_schedule_insns_after_reload = 0;
 
+  /* Variable tracking should be run after all optimizations which change order
+     of insns.  It also needs a valid CFG.  This can't be done in
+     bfin_option_override, because flag_var_tracking is finalized after
+     that.  */
+  bfin_flag_var_tracking = flag_var_tracking;
+  flag_var_tracking = 0;
+
   init_machine_status = bfin_init_machine_status;
 }
 
Index: gcc/config/ia64/ia64.c
===================================================================
--- gcc/config/ia64/ia64.c.orig	2011-03-31 03:58:25.493054332 -0300
+++ gcc/config/ia64/ia64.c	2011-03-31 03:58:55.176115781 -0300
@@ -2391,13 +2391,6 @@  ia64_expand_atomic_op (enum rtx_code cod
 static void
 ia64_file_start (void)
 {
-  /* Variable tracking should be run after all optimizations which change order
-     of insns.  It also needs a valid CFG.  This can't be done in
-     ia64_option_override, because flag_var_tracking is finalized after
-     that.  */
-  ia64_flag_var_tracking = flag_var_tracking;
-  flag_var_tracking = 0;
-
   default_file_start ();
   emit_safe_across_calls ();
 }
@@ -5722,6 +5715,13 @@  ia64_option_override (void)
   if (TARGET_ABI_OPEN_VMS)
     flag_no_common = 1;
 
+  /* Variable tracking should be run after all optimizations which change order
+     of insns.  It also needs a valid CFG.  This can't be done in
+     ia64_option_override, because flag_var_tracking is finalized after
+     that.  */
+  ia64_flag_var_tracking = flag_var_tracking;
+  flag_var_tracking = 0;
+
   ia64_override_options_after_change();
 }
 
Index: gcc/config/picochip/picochip.c
===================================================================
--- gcc/config/picochip/picochip.c.orig	2011-03-27 09:15:33.000000000 -0300
+++ gcc/config/picochip/picochip.c	2011-03-31 03:58:55.242115880 -0300
@@ -127,6 +127,7 @@  picochip_asm_named_section (const char *
 static rtx picochip_static_chain (const_tree, bool);
 
 static void picochip_option_override (void);
+static void picochip_override_options_after_change (void);
 
 /* Lookup table mapping a register number to the earliest containing
    class.  Used by REGNO_REG_CLASS.  */
@@ -335,7 +336,7 @@  static const struct default_options pico
 #define TARGET_OPTION_OVERRIDE picochip_option_override
 
 #undef TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE
-#define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE picochip_option_override
+#define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE picochip_override_options_after_change
 
 #undef TARGET_OPTION_OPTIMIZATION_TABLE
 #define TARGET_OPTION_OPTIMIZATION_TABLE picochip_option_optimization_table
@@ -356,14 +357,29 @@  picochip_return_in_memory(const_tree typ
   return ((unsigned HOST_WIDE_INT) int_size_in_bytes (type) > 4);
 }
 
-/* Allow some options to be overriden.  In particular, the 2nd
-   scheduling pass option is switched off, and a machine dependent
-   reorganisation ensures that it is run later on, after the second
-   jump optimisation. */
+/* Allow some options to be overriden.  */
 
 static void
 picochip_option_override (void)
 {
+  /* Variable tracking should be run after all optimizations which change order
+     of insns.  It also needs a valid CFG.  This can't be done in
+     picochip_option_override, because flag_var_tracking is finalized after
+     that.  */
+  picochip_flag_var_tracking = flag_var_tracking;
+  flag_var_tracking = 0;
+
+  picochip_override_options_after_change ();
+}
+
+/* Allow some options to be overriden after a configuration change.
+   In particular, the 2nd scheduling pass option is switched off, and
+   a machine dependent reorganisation ensures that it is run later on,
+   after the second jump optimisation. */
+
+static void
+picochip_override_options_after_change (void)
+{
   /* If we are optimizing for stack, dont let inliner to inline functions
      that could potentially increase stack size.*/
    if (flag_conserve_stack)
@@ -461,7 +477,6 @@  picochip_option_override (void)
 	error ("invalid mul type specified (%s) - expected mac, mul or none",
 	       picochip_mul_type_string);
     }
-
 }
 
 
@@ -1813,13 +1828,6 @@  picochip_asm_file_start (void)
     fprintf (asm_out_file, "// Has multiply: Yes (Mac unit)\n");
   else
     fprintf (asm_out_file, "// Has multiply: No\n");
-
-  /* Variable tracking should be run after all optimizations which change order
-     of insns.  It also needs a valid CFG.  This can't be done in
-     picochip_option_override, because flag_var_tracking is finalized after
-     that.  */
-  picochip_flag_var_tracking = flag_var_tracking;
-  flag_var_tracking = 0;
 }
 
 /* Output the end of an ASM file. */
Index: gcc/config/spu/spu.c
===================================================================
--- gcc/config/spu/spu.c.orig	2011-03-27 09:15:57.000000000 -0300
+++ gcc/config/spu/spu.c	2011-03-31 03:58:55.331116013 -0300
@@ -573,6 +573,19 @@  spu_option_override (void)
     }
 
   REAL_MODE_FORMAT (SFmode) = &spu_single_format;
+
+  /* Variable tracking should be run after all optimizations which
+     change order of insns.  It also needs a valid CFG.  Therefore,
+     *if* we make nontrivial changes in machine-dependent reorg,
+     run variable tracking after those.  However, if we do not run
+     our machine-dependent reorg pass, we must still run the normal
+     variable tracking pass (or else we will ICE in final since
+     debug insns have not been removed).  */
+  if (TARGET_BRANCH_HINTS && optimize)
+    {
+      spu_flag_var_tracking = flag_var_tracking;
+      flag_var_tracking = 0;
+    }
 }
 
 /* Handle an attribute requiring a FUNCTION_DECL; arguments as in
@@ -7052,19 +7065,6 @@  spu_libgcc_shift_count_mode (void)
 static void
 asm_file_start (void)
 {
-  /* Variable tracking should be run after all optimizations which
-     change order of insns.  It also needs a valid CFG.  Therefore,
-     *if* we make nontrivial changes in machine-dependent reorg,
-     run variable tracking after those.  However, if we do not run
-     our machine-dependent reorg pass, we must still run the normal
-     variable tracking pass (or else we will ICE in final since
-     debug insns have not been removed).  */
-  if (TARGET_BRANCH_HINTS && optimize)
-    {
-      spu_flag_var_tracking = flag_var_tracking;
-      flag_var_tracking = 0;
-    }
-
   default_file_start ();
 }