* [PATCH] Release virtual SSA names properly
@ 2012-02-10 14:08 Richard Guenther
2012-02-10 15:36 ` Richard Guenther
0 siblings, 1 reply; 2+ messages in thread
From: Richard Guenther @ 2012-02-10 14:08 UTC (permalink / raw)
To: gcc-patches
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).
Thanks,
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;
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Release virtual SSA names properly
2012-02-10 14:08 [PATCH] Release virtual SSA names properly Richard Guenther
@ 2012-02-10 15:36 ` Richard Guenther
0 siblings, 0 replies; 2+ messages in thread
From: Richard Guenther @ 2012-02-10 15:36 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: TEXT/PLAIN, Size: 8167 bytes --]
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-02-10 15:35 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-10 14:08 [PATCH] Release virtual SSA names properly Richard Guenther
2012-02-10 15:36 ` Richard Guenther
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).