diff mbox

[4.5] Fix PR 44914

Message ID 20100802204104.GD18246@virgil.arch.suse.de
State New
Headers show

Commit Message

Martin Jambor Aug. 2, 2010, 8:41 p.m. UTC
Hi,

fix for PR 44914 on the 4.5 branch needs to be a little bit different
because of how the functions are organized there.  In particular,
scan_function is not split into three different ones and it already
returns a value.  Therefore I resorted to a global variable to flag
that CFG has been changed.  This is a bit ugly but it is not the first
global variable that we use (there already is
encountered_unchangable_recursive_call) and it is fortunately used
only in a very simple way.

Bootstrapped and tested on the branch on x86_64-linux, OK for trunk?

Thanks,

Martin


2010-08-02  Martin Jambor  <mjambor@suse.cz>

	PR tree-optimization/44914
	* tree-sra.c (cfg_changed): New variable.
	(sra_initialize): Initialize cgf_changed to false.
	(scan_function): Set cfg_changed if cfg was changed.
	(perform_intra_sra): Return also TODO_update_sa if cfg was changed.
	(ipa_early_sra): Likewise.

	* testsuite/g++.dg/tree-ssa/pr44914.C: New test

Comments

Richard Biener Aug. 3, 2010, 8:45 a.m. UTC | #1
On Mon, 2 Aug 2010, Martin Jambor wrote:

> Hi,
> 
> fix for PR 44914 on the 4.5 branch needs to be a little bit different
> because of how the functions are organized there.  In particular,
> scan_function is not split into three different ones and it already
> returns a value.  Therefore I resorted to a global variable to flag
> that CFG has been changed.  This is a bit ugly but it is not the first
> global variable that we use (there already is
> encountered_unchangable_recursive_call) and it is fortunately used
> only in a very simple way.
> 
> Bootstrapped and tested on the branch on x86_64-linux, OK for trunk?

Ok for the branch.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 2010-08-02  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR tree-optimization/44914
> 	* tree-sra.c (cfg_changed): New variable.
> 	(sra_initialize): Initialize cgf_changed to false.
> 	(scan_function): Set cfg_changed if cfg was changed.
> 	(perform_intra_sra): Return also TODO_update_sa if cfg was changed.
> 	(ipa_early_sra): Likewise.
> 
> 	* testsuite/g++.dg/tree-ssa/pr44914.C: New test
> 
> Index: gcc-4_5-branch/gcc/tree-sra.c
> ===================================================================
> --- gcc-4_5-branch.orig/gcc/tree-sra.c
> +++ gcc-4_5-branch/gcc/tree-sra.c
> @@ -276,6 +276,9 @@ static bool encountered_apply_args;
>     arguments than formal parameters..  */
>  static bool encountered_unchangable_recursive_call;
>  
> +/* Set by scan_function when it changes the control flow graph.  */
> +static bool cfg_changed;
> +
>  /* This is a table in which for each basic block and parameter there is a
>     distance (offset + size) in that parameter which is dereferenced and
>     accessed in that BB.  */
> @@ -570,6 +573,7 @@ sra_initialize (void)
>    memset (&sra_stats, 0, sizeof (sra_stats));
>    encountered_apply_args = false;
>    encountered_unchangable_recursive_call = false;
> +  cfg_changed = false;
>  }
>  
>  /* Hook fed to pointer_map_traverse, deallocate stored vectors.  */
> @@ -1114,8 +1118,6 @@ scan_function (bool (*scan_expr) (tree *
>  
>    FOR_EACH_BB (bb)
>      {
> -      bool bb_changed = false;
> -
>        if (handle_ssa_defs)
>  	for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>  	  ret |= handle_ssa_defs (gsi_stmt (gsi), data);
> @@ -1220,21 +1222,15 @@ scan_function (bool (*scan_expr) (tree *
>  
>  	      if (!analysis_stage)
>  		{
> -		  bb_changed = true;
>  		  update_stmt (stmt);
> -		  maybe_clean_eh_stmt (stmt);
> +		  if (maybe_clean_eh_stmt (stmt)
> +		      && gimple_purge_dead_eh_edges (bb))
> +		    cfg_changed = true;
>  		}
>  	    }
> -	  if (deleted)
> -	    bb_changed = true;
> -	  else
> -	    {
> -	      gsi_next (&gsi);
> -	      ret = true;
> -	    }
> +	  if (!deleted)
> +	    gsi_next (&gsi);
>  	}
> -      if (!analysis_stage && bb_changed && sra_mode == SRA_MODE_EARLY_IPA)
> -	gimple_purge_dead_eh_edges (bb);
>      }
>  
>    return ret;
> @@ -2871,7 +2867,10 @@ perform_intra_sra (void)
>    statistics_counter_event (cfun, "Separate LHS and RHS handling",
>  			    sra_stats.separate_lhs_rhs_handling);
>  
> -  ret = TODO_update_ssa;
> +  if (cfg_changed)
> +    ret = TODO_update_ssa | TODO_cleanup_cfg;
> +  else
> +    ret = TODO_update_ssa;
>  
>   out:
>    sra_deinitialize ();
> @@ -4236,7 +4235,10 @@ ipa_early_sra (void)
>  
>    modify_function (node, adjustments);
>    VEC_free (ipa_parm_adjustment_t, heap, adjustments);
> -  ret = TODO_update_ssa;
> +  if (cfg_changed)
> +    ret = TODO_update_ssa | TODO_cleanup_cfg;
> +  else
> +    ret = TODO_update_ssa;
>  
>    statistics_counter_event (cfun, "Unused parameters deleted",
>  			    sra_stats.deleted_unused_parameters);
> Index: gcc-4_5-branch/gcc/testsuite/g++.dg/tree-ssa/pr44914.C
> ===================================================================
> --- /dev/null
> +++ gcc-4_5-branch/gcc/testsuite/g++.dg/tree-ssa/pr44914.C
> @@ -0,0 +1,20 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O1 -fipa-sra -fnon-call-exceptions" } */
> +
> +struct A
> +{
> +  ~A () { }
> +};
> +
> +struct B
> +{
> +  A a;
> +  int i;
> +  void f (int) { }
> +  B ()
> +  {
> +    f (i);
> +  }
> +};
> +
> +B b;
> 
>
diff mbox

Patch

Index: gcc-4_5-branch/gcc/tree-sra.c
===================================================================
--- gcc-4_5-branch.orig/gcc/tree-sra.c
+++ gcc-4_5-branch/gcc/tree-sra.c
@@ -276,6 +276,9 @@  static bool encountered_apply_args;
    arguments than formal parameters..  */
 static bool encountered_unchangable_recursive_call;
 
+/* Set by scan_function when it changes the control flow graph.  */
+static bool cfg_changed;
+
 /* This is a table in which for each basic block and parameter there is a
    distance (offset + size) in that parameter which is dereferenced and
    accessed in that BB.  */
@@ -570,6 +573,7 @@  sra_initialize (void)
   memset (&sra_stats, 0, sizeof (sra_stats));
   encountered_apply_args = false;
   encountered_unchangable_recursive_call = false;
+  cfg_changed = false;
 }
 
 /* Hook fed to pointer_map_traverse, deallocate stored vectors.  */
@@ -1114,8 +1118,6 @@  scan_function (bool (*scan_expr) (tree *
 
   FOR_EACH_BB (bb)
     {
-      bool bb_changed = false;
-
       if (handle_ssa_defs)
 	for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
 	  ret |= handle_ssa_defs (gsi_stmt (gsi), data);
@@ -1220,21 +1222,15 @@  scan_function (bool (*scan_expr) (tree *
 
 	      if (!analysis_stage)
 		{
-		  bb_changed = true;
 		  update_stmt (stmt);
-		  maybe_clean_eh_stmt (stmt);
+		  if (maybe_clean_eh_stmt (stmt)
+		      && gimple_purge_dead_eh_edges (bb))
+		    cfg_changed = true;
 		}
 	    }
-	  if (deleted)
-	    bb_changed = true;
-	  else
-	    {
-	      gsi_next (&gsi);
-	      ret = true;
-	    }
+	  if (!deleted)
+	    gsi_next (&gsi);
 	}
-      if (!analysis_stage && bb_changed && sra_mode == SRA_MODE_EARLY_IPA)
-	gimple_purge_dead_eh_edges (bb);
     }
 
   return ret;
@@ -2871,7 +2867,10 @@  perform_intra_sra (void)
   statistics_counter_event (cfun, "Separate LHS and RHS handling",
 			    sra_stats.separate_lhs_rhs_handling);
 
-  ret = TODO_update_ssa;
+  if (cfg_changed)
+    ret = TODO_update_ssa | TODO_cleanup_cfg;
+  else
+    ret = TODO_update_ssa;
 
  out:
   sra_deinitialize ();
@@ -4236,7 +4235,10 @@  ipa_early_sra (void)
 
   modify_function (node, adjustments);
   VEC_free (ipa_parm_adjustment_t, heap, adjustments);
-  ret = TODO_update_ssa;
+  if (cfg_changed)
+    ret = TODO_update_ssa | TODO_cleanup_cfg;
+  else
+    ret = TODO_update_ssa;
 
   statistics_counter_event (cfun, "Unused parameters deleted",
 			    sra_stats.deleted_unused_parameters);
Index: gcc-4_5-branch/gcc/testsuite/g++.dg/tree-ssa/pr44914.C
===================================================================
--- /dev/null
+++ gcc-4_5-branch/gcc/testsuite/g++.dg/tree-ssa/pr44914.C
@@ -0,0 +1,20 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O1 -fipa-sra -fnon-call-exceptions" } */
+
+struct A
+{
+  ~A () { }
+};
+
+struct B
+{
+  A a;
+  int i;
+  void f (int) { }
+  B ()
+  {
+    f (i);
+  }
+};
+
+B b;