Re: [PATCH, 4.5] Fix PR 44914

[Richard Guenther <rguenther@suse.de>]

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;
> 
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex