This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Release virtual SSA names properly


On Fri, 10 Feb 2012, Richard Guenther wrote:

> 
> I've played with
> 
> Index: tree-ssa.c
> ===================================================================
> --- tree-ssa.c  (revision 184088)
> +++ tree-ssa.c  (working copy)
> @@ -944,6 +944,11 @@ verify_ssa (bool check_modified_stmt)
>           if (!gimple_nop_p (stmt))
>             {
>               basic_block bb = gimple_bb (stmt);
> +             if (!bb)
> +               {
> +                 error ("Unreleased SSA name");
> +                 goto err;
> +               }
>               verify_def (bb, definition_block,
>                           name, stmt, !is_gimple_reg (name));
>  
> and noticed we do not properly free a lot of virtual SSA names, leaking
> both SSA names, stmts and operands they point to over time.
> 
> The following patch cures the cases I hit with running the tree-ssa.exp
> testsuite.  More issues exist, and I guess we should make a more
> verbose variant of the above the default in 4.8 to catch these issues
> (there is a slight issue with this when verify_ssa is invoked during
> loop transforms and the vectorizer has pattern stmts built that are
> not in a basic block ... so maybe we should put the above into some
> other verifier).
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in currently in
> progress.
> 
> I'll commit this unless somebody thinks it's a really bad idea
> at this point (I do not have numbers for the amount of memory
> we save - once we have fully compiled a function and we do not
> need its body for further inlining we throw it away and with it
> all leaked SSA names).

I'm not installing this now because I hit PR52198 with this change.
I'm instead queuing it for 4.8 where I can XFAIL the testcases
that are affected.

Richard.

> 2012-02-10  Richard Guenther  <rguenther@suse.de>
> 
> 	* tree-nrv.c (tree_nrv): Release VDEFs.
> 	* tree-inline.c (expand_call_inline): Likewise.
> 	* tree-sra.c (sra_modify_constructor_assign): Likewise.
> 	(sra_modify_assign): Likewise.
> 	* tree-vect-stmts.c (vect_remove_stores): Likewise.
> 	* tree-vect-loop.c (vect_transform_loop): Likewise.
> 	* tree-ssa-dom.c (optimize_stmt): Likewise.
> 	* tree-vect-slp.c (vect_schedule_slp): Likewise.
> 	* tree-ssa-math-opts.c (execute_cse_sincos): Likewise.
> 
> Index: gcc/tree-nrv.c
> ===================================================================
> --- gcc/tree-nrv.c	(revision 184088)
> +++ gcc/tree-nrv.c	(working copy)
> @@ -244,6 +244,7 @@ tree_nrv (void)
>  	    {
>  	      unlink_stmt_vdef (stmt);
>  	      gsi_remove (&gsi, true);
> +	      release_defs (stmt);
>  	    }
>  	  else
>  	    {
> Index: gcc/tree-inline.c
> ===================================================================
> --- gcc/tree-inline.c	(revision 184088)
> +++ gcc/tree-inline.c	(working copy)
> @@ -4002,6 +4002,9 @@ expand_call_inline (basic_block bb, gimp
>  
>    /* Unlink the calls virtual operands before replacing it.  */
>    unlink_stmt_vdef (stmt);
> +  if (gimple_vdef (stmt)
> +      && TREE_CODE (gimple_vdef (stmt)) == SSA_NAME)
> +    release_ssa_name (gimple_vdef (stmt));
>  
>    /* If the inlined function returns a result that we care about,
>       substitute the GIMPLE_CALL with an assignment of the return
> @@ -4043,7 +4046,7 @@ expand_call_inline (basic_block bb, gimp
>  	    }
>  	}
>        else
> -        gsi_remove (&stmt_gsi, true);
> +	gsi_remove (&stmt_gsi, true);
>      }
>  
>    if (purge_dead_abnormal_edges)
> Index: gcc/tree-sra.c
> ===================================================================
> --- gcc/tree-sra.c	(revision 184088)
> +++ gcc/tree-sra.c	(working copy)
> @@ -2858,6 +2858,7 @@ sra_modify_constructor_assign (gimple *s
>  	{
>  	  unlink_stmt_vdef (*stmt);
>  	  gsi_remove (gsi, true);
> +	  release_defs (*stmt);
>  	  return SRA_AM_REMOVED;
>  	}
>        else
> @@ -2881,6 +2882,7 @@ sra_modify_constructor_assign (gimple *s
>        init_subtree_with_zero (acc, gsi, false, loc);
>        unlink_stmt_vdef (*stmt);
>        gsi_remove (gsi, true);
> +      release_defs (*stmt);
>        return SRA_AM_REMOVED;
>      }
>    else
> @@ -3125,6 +3127,7 @@ sra_modify_assign (gimple *stmt, gimple_
>  	      gsi_next (gsi);
>  	      unlink_stmt_vdef (*stmt);
>  	      gsi_remove (&orig_gsi, true);
> +	      release_defs (*stmt);
>  	      sra_stats.deleted++;
>  	      return SRA_AM_REMOVED;
>  	    }
> @@ -3145,6 +3148,7 @@ sra_modify_assign (gimple *stmt, gimple_
>  	      gcc_assert (*stmt == gsi_stmt (*gsi));
>  	      unlink_stmt_vdef (*stmt);
>  	      gsi_remove (gsi, true);
> +	      release_defs (*stmt);
>  	      sra_stats.deleted++;
>  	      return SRA_AM_REMOVED;
>  	    }
> Index: gcc/tree-vect-stmts.c
> ===================================================================
> --- gcc/tree-vect-stmts.c	(revision 184088)
> +++ gcc/tree-vect-stmts.c	(working copy)
> @@ -5596,7 +5596,9 @@ vect_remove_stores (gimple first_stmt)
>  	next = STMT_VINFO_RELATED_STMT (stmt_info);
>        /* Free the attached stmt_vec_info and remove the stmt.  */
>        next_si = gsi_for_stmt (next);
> +      unlink_stmt_vdef (next);
>        gsi_remove (&next_si, true);
> +      release_defs (next);
>        free_stmt_vec_info (next);
>        next = tmp;
>      }
> Index: gcc/tree-vect-loop.c
> ===================================================================
> --- gcc/tree-vect-loop.c	(revision 184088)
> +++ gcc/tree-vect-loop.c	(working copy)
> @@ -5428,8 +5428,11 @@ vect_transform_loop (loop_vec_info loop_
>  	      else
>  		{
>  		  /* Free the attached stmt_vec_info and remove the stmt.  */
> -		  free_stmt_vec_info (gsi_stmt (si));
> +		  gcc_assert (stmt == gsi_stmt (si));
> +		  free_stmt_vec_info (stmt);
> +		  unlink_stmt_vdef (stmt);
>  		  gsi_remove (&si, true);
> +		  release_defs (stmt);
>  		  continue;
>  		}
>  	    }
> Index: gcc/tree-ssa-dom.c
> ===================================================================
> --- gcc/tree-ssa-dom.c	(revision 184088)
> +++ gcc/tree-ssa-dom.c	(working copy)
> @@ -2294,6 +2294,7 @@ optimize_stmt (basic_block bb, gimple_st
>  	      int lp_nr = lookup_stmt_eh_lp (stmt);
>  	      unlink_stmt_vdef (stmt);
>  	      gsi_remove (&si, true);
> +	      release_defs (stmt);
>  	      if (lp_nr != 0)
>  		{
>  		  bitmap_set_bit (need_eh_cleanup, bb->index);
> Index: gcc/tree-vect-slp.c
> ===================================================================
> --- gcc/tree-vect-slp.c	(revision 184088)
> +++ gcc/tree-vect-slp.c	(working copy)
> @@ -3008,7 +3008,9 @@ vect_schedule_slp (loop_vec_info loop_vi
>             store = STMT_VINFO_RELATED_STMT (vinfo_for_stmt (store));
>            /* Free the attached stmt_vec_info and remove the stmt.  */
>            gsi = gsi_for_stmt (store);
> +	  unlink_stmt_vdef (store);
>            gsi_remove (&gsi, true);
> +	  release_defs (store);
>            free_stmt_vec_info (store);
>          }
>      }
> Index: gcc/tree-ssa-math-opts.c
> ===================================================================
> --- gcc/tree-ssa-math-opts.c	(revision 184088)
> +++ gcc/tree-ssa-math-opts.c	(working copy)
> @@ -1430,6 +1430,8 @@ execute_cse_sincos (void)
>  		      gimple_set_location (new_stmt, loc);
>  		      unlink_stmt_vdef (stmt);
>  		      gsi_replace (&gsi, new_stmt, true);
> +		      if (gimple_vdef (stmt))
> +			release_ssa_name (gimple_vdef (stmt));
>  		    }
>  		  break;
>  
> @@ -1450,6 +1452,8 @@ execute_cse_sincos (void)
>  		      gimple_set_location (new_stmt, loc);
>  		      unlink_stmt_vdef (stmt);
>  		      gsi_replace (&gsi, new_stmt, true);
> +		      if (gimple_vdef (stmt))
> +			release_ssa_name (gimple_vdef (stmt));
>  		    }
>  		  break;
>  
> @@ -1465,6 +1469,8 @@ execute_cse_sincos (void)
>  		      gimple_set_location (new_stmt, loc);
>  		      unlink_stmt_vdef (stmt);
>  		      gsi_replace (&gsi, new_stmt, true);
> +		      if (gimple_vdef (stmt))
> +			release_ssa_name (gimple_vdef (stmt));
>  		    }
>  		  break;
>  
> 

-- 
Richard Guenther <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]