* [PATCH] Minor refactoring in tree-ssanames.c & freelists verifier
@ 2015-11-09 9:02 Jeff Law
2015-11-09 15:00 ` Michael Matz
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2015-11-09 9:02 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]
This is a piece of a larger refactoring. I'm postponing the larger
refactoring until next stage1 -- I have some concerns about the
compile-time impact that I'll need to measure.
In the mean time I want to get knowledge of the freelists out of
pass_release_ssa_names::execute. The larger refactoring will move the
freelists into a class and pass_release_ssa_names won't have access to
those private members. So the guts of that routine have been moved into
what will ultimately be a method of the name manager.
I'm also including a leak detector/verification routine for the
freelists. This version will bootstrap on the trunk if the verifier is
enabled at the end of flush_ssanames_freelist. However, the vectorizer
leaks names horribly, thus it won't pass a regression test or a -O3
bootstrap. In the mean time, the verifier is still useful to help find
leaks -- it's obviously not checking by default and the code is marked
as a debug function.
This wraps up the things I expect to address during this stage1 cycle.
I've got a fair amount of patch review to cover in the immediate future
as well as starting the bugfixing cycle.
Bootstrapped and regression tested on x86_64-linux-gnu. Installed on
the trunk.
Jeff
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 11182 bytes --]
commit feba260ed75060c9acc265ff12a44557e280f5be
Author: Jeff Law <law@redhat.com>
Date: Sun Nov 8 20:33:59 2015 -0700
[PATCH] Minor refactoring in tree-ssanames.c & freelists verifier
* tree-into-ssa.c (names_to_release): No longer static.
* tree-into-ssa.h (names_to_release): Declare.
* tree-ssanames.c (verify_ssaname_freelists): New debug function.
(release_free_names_and_compact_live_names): New function extracted
from pass_release_ssa_names::execute.
(pass_release_ssa_names::execute): Use it.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index dfb8350..c70ab87 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2015-11-09 Jeff Law <law@redhat.com>
+
+ * tree-into-ssa.c (names_to_release): No longer static.
+ * tree-into-ssa.h (names_to_release): Declare.
+ * tree-ssanames.c (verify_ssaname_freelists): New debug function.
+ (release_free_names_and_compact_live_names): New function extracted
+ from pass_release_ssa_names::execute.
+ (pass_release_ssa_names::execute): Use it.
+
2015-11-09 Alan Modra <amodra@gmail.com>
* gensupport.c (add_mnemonic_string): Make len param a size_t.
@@ -11,7 +20,7 @@
* gcc/bb-reorder.c (reorder_basic_blocks_simple): Treat a conditional
branch with only one successor just like unconditional branches.
-2015-11-08 Jeff Law <jeff@redhat.com>
+2015-11-08 Jeff Law <law@redhat.com>
* tree-ssa-threadupdate.c (register_jump_thraed): Assert that a
non-FSM path has no edges marked with EDGE_DFS_BACK.
@@ -369,7 +378,7 @@
the dominance info; free it if we can't.
(pass_call_cdce::execute): Don't free the dominance info here.
-2015-11-06 Jeff Law <jeff@redhat.com>
+2015-11-06 Jeff Law <law@redhat.com>
* tree-ssa-threadedge.c (dummy_simplify): Remove.
(thread_around_empty_blocks): Remove backedge_seen_p argument.
@@ -402,7 +411,7 @@
(build_scop_scattering): Call build_pbb_minimal_scattering_polyhedrons.
(build_poly_scop): Call build_scop_minimal_scattering.
-2015-11-06 Jeff Law <jeff@redhat.com>
+2015-11-06 Jeff Law <law@redhat.com>
* cfg-flags.def (IGNORE): New edge flag.
* tree-vrp.c (identify_jump_threads): Mark and clear edges
@@ -1174,7 +1183,7 @@
* tree-ssa.c (gimple_uses_undefined_value_p): New.
* tree-ssa.h (gimple_uses_undefined_value_p): Declare.
-2015-11-02 Jeff Law <jeff@redhat.com>
+2015-11-02 Jeff Law <law@redhat.com>
* tree-ssa-threadupdate.c (valid_jump_thread_path): Also detect
cases where the loop latch edge is in the middle of an FSM path.
@@ -1238,7 +1247,7 @@
PR middle-end/68166
* fold-const.c: Include "md5.h".
-2015-11-01 Jeff Law <jeff@redhat.com>
+2015-11-01 Jeff Law <law@redhat.com>
* vmsdbgout.c: Revert unused header file reduction patch.
@@ -1287,7 +1296,7 @@
* tree-ssa-structalias.c (ipa_pta_execute): Use make_copy_constraint.
-2015-10-30 Jeff Law <jeff@redhat.com>
+2015-10-30 Jeff Law <law@redhat.com>
Nathan Sidwell <nathan@acm.org>
* config/nvptx/nvptx.h (HARD_REGNO_NREGS): Avoid warning on unused
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index 6533998..3086f82 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -94,7 +94,7 @@ static sbitmap interesting_blocks;
/* Set of SSA names that have been marked to be released after they
were registered in the replacement table. They will be finally
released after we finish updating the SSA web. */
-static bitmap names_to_release;
+bitmap names_to_release;
/* vec of vec of PHIs to rewrite in a basic block. Element I corresponds
the to basic block with index I. Allocated once per compilation, *not*
diff --git a/gcc/tree-into-ssa.h b/gcc/tree-into-ssa.h
index c053f78..aed1e95 100644
--- a/gcc/tree-into-ssa.h
+++ b/gcc/tree-into-ssa.h
@@ -48,5 +48,6 @@ extern void dump_names_replaced_by (FILE *, tree);
extern void debug_names_replaced_by (tree);
extern void dump_update_ssa (FILE *);
extern void debug_update_ssa (void);
+extern bitmap names_to_release;
#endif /* GCC_TREE_INTO_SSA_H */
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 12235f6..096b75b 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -114,6 +114,133 @@ ssanames_print_statistics (void)
fprintf (stderr, "SSA_NAME nodes reused: %u\n", ssa_name_nodes_reused);
}
+/* Verify the state of the SSA_NAME lists.
+
+ There must be no duplicates on the free list.
+ Every name on the free list must be marked as on the free list.
+ Any name on the free list must not appear in the IL.
+ No names can be leaked. */
+
+DEBUG_FUNCTION void
+verify_ssaname_freelists (struct function *fun)
+{
+ /* Do nothing if we are in RTL format. */
+ basic_block bb;
+ FOR_EACH_BB_FN (bb, fun)
+ {
+ if (bb->flags & BB_RTL)
+ return;
+ }
+
+ bitmap names_in_il = BITMAP_ALLOC (NULL);
+
+ /* Walk the entire IL noting every SSA_NAME we see. */
+ FOR_EACH_BB_FN (bb, fun)
+ {
+ tree t;
+ /* First note the result and arguments of PHI nodes. */
+ for (gphi_iterator gsi = gsi_start_phis (bb);
+ !gsi_end_p (gsi);
+ gsi_next (&gsi))
+ {
+ gphi *phi = gsi.phi ();
+ t = gimple_phi_result (phi);
+ bitmap_set_bit (names_in_il, SSA_NAME_VERSION (t));
+
+ for (unsigned int i = 0; i < gimple_phi_num_args (phi); i++)
+ {
+ t = gimple_phi_arg_def (phi, i);
+ if (TREE_CODE (t) == SSA_NAME)
+ bitmap_set_bit (names_in_il, SSA_NAME_VERSION (t));
+ }
+ }
+
+ /* Then note the operands of each statement. */
+ for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
+ !gsi_end_p (gsi);
+ gsi_next (&gsi))
+ {
+ ssa_op_iter iter;
+ gimple *stmt = gsi_stmt (gsi);
+ FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
+ if (TREE_CODE (t) == SSA_NAME)
+ bitmap_set_bit (names_in_il, SSA_NAME_VERSION (t));
+ }
+ }
+
+ /* Now walk the free list noting what we find there and verifying
+ there are no duplicates. */
+ bitmap names_in_freelists = BITMAP_ALLOC (NULL);
+ if (FREE_SSANAMES (fun))
+ {
+ for (unsigned int i = 0; i < FREE_SSANAMES (fun)->length (); i++)
+ {
+ tree t = (*FREE_SSANAMES (fun))[i];
+
+ /* Verify that the name is marked as being in the free list. */
+ gcc_assert (SSA_NAME_IN_FREE_LIST (t));
+
+ /* Verify the name has not already appeared in the free list and
+ note it in the list of names found in the free list. */
+ gcc_assert (!bitmap_bit_p (names_in_freelists, SSA_NAME_VERSION (t)));
+ bitmap_set_bit (names_in_freelists, SSA_NAME_VERSION (t));
+ }
+ }
+
+ /* Similarly for the names in the pending free list. */
+ if (FREE_SSANAMES_QUEUE (fun))
+ {
+ for (unsigned int i = 0; i < FREE_SSANAMES_QUEUE (fun)->length (); i++)
+ {
+ tree t = (*FREE_SSANAMES_QUEUE (fun))[i];
+
+ /* Verify that the name is marked as being in the free list. */
+ gcc_assert (SSA_NAME_IN_FREE_LIST (t));
+
+ /* Verify the name has not already appeared in the free list and
+ note it in the list of names found in the free list. */
+ gcc_assert (!bitmap_bit_p (names_in_freelists, SSA_NAME_VERSION (t)));
+ bitmap_set_bit (names_in_freelists, SSA_NAME_VERSION (t));
+ }
+ }
+
+ /* If any name appears in both the IL and the freelists, then
+ something horrible has happened. */
+ bool intersect_p = bitmap_intersect_p (names_in_il, names_in_freelists);
+ gcc_assert (!intersect_p);
+
+ /* Names can be queued up for release if there is an ssa update
+ pending. Pretend we saw them in the IL. */
+ if (names_to_release)
+ bitmap_ior_into (names_in_il, names_to_release);
+
+ /* Function splitting can "lose" SSA_NAMEs in an effort to ensure that
+ debug/non-debug compilations have the same SSA_NAMEs. So for each
+ lost SSA_NAME, see if it's likely one from that wart. These will always
+ be marked as default definitions. So we loosely assume that anything
+ marked as a default definition isn't leaked by pretening they are
+ in the IL. */
+ for (unsigned int i = UNUSED_NAME_VERSION + 1; i < num_ssa_names; i++)
+ if (ssa_name (i) && SSA_NAME_IS_DEFAULT_DEF (ssa_name (i)))
+ bitmap_set_bit (names_in_il, i);
+
+ unsigned int i;
+ bitmap_iterator bi;
+ bitmap all_names = BITMAP_ALLOC (NULL);
+ bitmap_set_range (all_names, UNUSED_NAME_VERSION + 1, num_ssa_names - 1);
+ bitmap_ior_into (names_in_il, names_in_freelists);
+
+ /* Any name not mentioned in the IL and not in the feelists
+ has been leaked. */
+ EXECUTE_IF_AND_COMPL_IN_BITMAP(all_names, names_in_il,
+ UNUSED_NAME_VERSION + 1, i, bi)
+ gcc_assert (!ssa_name (i));
+
+ BITMAP_FREE (all_names);
+ BITMAP_FREE (names_in_freelists);
+ BITMAP_FREE (names_in_il);
+}
+
/* Move all SSA_NAMEs from FREE_SSA_NAMES_QUEUE to FREE_SSA_NAMES.
We do not, but should have a mode to verify the state of the SSA_NAMEs
@@ -604,6 +731,42 @@ replace_ssa_name_symbol (tree ssa_name, tree sym)
TREE_TYPE (ssa_name) = TREE_TYPE (sym);
}
+/* Release the vector of free SSA_NAMEs and compact the the
+ vector of SSA_NAMEs that are live. */
+
+static void
+release_free_names_and_compact_live_names (function *fun)
+{
+ unsigned i, j;
+ int n = vec_safe_length (FREE_SSANAMES (fun));
+
+ /* Now release the freelist. */
+ vec_free (FREE_SSANAMES (fun));
+
+ /* And compact the SSA number space. We make sure to not change the
+ relative order of SSA versions. */
+ for (i = 1, j = 1; i < fun->gimple_df->ssa_names->length (); ++i)
+ {
+ tree name = ssa_name (i);
+ if (name)
+ {
+ if (i != j)
+ {
+ SSA_NAME_VERSION (name) = j;
+ (*fun->gimple_df->ssa_names)[j] = name;
+ }
+ j++;
+ }
+ }
+ fun->gimple_df->ssa_names->truncate (j);
+
+ statistics_counter_event (fun, "SSA names released", n);
+ statistics_counter_event (fun, "SSA name holes removed", i - j);
+ if (dump_file)
+ fprintf (dump_file, "Released %i names, %.2f%%, removed %i holes\n",
+ n, n * 100.0 / num_ssa_names, i - j);
+}
+
/* Return SSA names that are unused to GGC memory and compact the SSA
version namespace. This is used to keep footprint of compiler during
interprocedural optimization. */
@@ -638,34 +801,7 @@ public:
unsigned int
pass_release_ssa_names::execute (function *fun)
{
- unsigned i, j;
- int n = vec_safe_length (FREE_SSANAMES (fun));
-
- /* Now release the freelist. */
- vec_free (FREE_SSANAMES (fun));
-
- /* And compact the SSA number space. We make sure to not change the
- relative order of SSA versions. */
- for (i = 1, j = 1; i < fun->gimple_df->ssa_names->length (); ++i)
- {
- tree name = ssa_name (i);
- if (name)
- {
- if (i != j)
- {
- SSA_NAME_VERSION (name) = j;
- (*fun->gimple_df->ssa_names)[j] = name;
- }
- j++;
- }
- }
- fun->gimple_df->ssa_names->truncate (j);
-
- statistics_counter_event (fun, "SSA names released", n);
- statistics_counter_event (fun, "SSA name holes removed", i - j);
- if (dump_file)
- fprintf (dump_file, "Released %i names, %.2f%%, removed %i holes\n",
- n, n * 100.0 / num_ssa_names, i - j);
+ release_free_names_and_compact_live_names (fun);
return 0;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Minor refactoring in tree-ssanames.c & freelists verifier
2015-11-09 9:02 [PATCH] Minor refactoring in tree-ssanames.c & freelists verifier Jeff Law
@ 2015-11-09 15:00 ` Michael Matz
2015-11-09 19:59 ` Jeff Law
0 siblings, 1 reply; 3+ messages in thread
From: Michael Matz @ 2015-11-09 15:00 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
Hi,
On Mon, 9 Nov 2015, Jeff Law wrote:
+verify_ssaname_freelists (struct function *fun)
+{
+ /* Do nothing if we are in RTL format. */
+ basic_block bb;
+ FOR_EACH_BB_FN (bb, fun)
+ {
+ if (bb->flags & BB_RTL)
+ return;
+ }
gimple_in_ssa_p (fun);
+ /* Then note the operands of each statement. */
+ for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
+ !gsi_end_p (gsi);
+ gsi_next (&gsi))
+ {
+ ssa_op_iter iter;
+ gimple *stmt = gsi_stmt (gsi);
+ FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
+ if (TREE_CODE (t) == SSA_NAME)
+ bitmap_set_bit (names_in_il, SSA_NAME_VERSION (t));
+ }
t will always be an SSA_NAME here.
Ciao,
Michael.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Minor refactoring in tree-ssanames.c & freelists verifier
2015-11-09 15:00 ` Michael Matz
@ 2015-11-09 19:59 ` Jeff Law
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Law @ 2015-11-09 19:59 UTC (permalink / raw)
To: Michael Matz; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]
On 11/09/2015 08:00 AM, Michael Matz wrote:
> Hi,
>
> On Mon, 9 Nov 2015, Jeff Law wrote:
>
> +verify_ssaname_freelists (struct function *fun)
> +{
> + /* Do nothing if we are in RTL format. */
> + basic_block bb;
> + FOR_EACH_BB_FN (bb, fun)
> + {
> + if (bb->flags & BB_RTL)
> + return;
> + }
>
> gimple_in_ssa_p (fun);
Agreed & fixed.
>
> + /* Then note the operands of each statement. */
> + for (gimple_stmt_iterator gsi = gsi_start_bb (bb);
> + !gsi_end_p (gsi);
> + gsi_next (&gsi))
> + {
> + ssa_op_iter iter;
> + gimple *stmt = gsi_stmt (gsi);
> + FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
> + if (TREE_CODE (t) == SSA_NAME)
> + bitmap_set_bit (names_in_il, SSA_NAME_VERSION (t));
> + }
>
> t will always be an SSA_NAME here.
Likewise. I think that test was in there from a time when I'd run the
verifier at a different point in the pipeline and things weren't
necessarily consistent. I'll simplify in the obvious way.
I put bootstrapped x86_64-linux-gnu with the verifier enabled.
Installed on the trunk.
Thanks for catching these.
Jeff
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 2538 bytes --]
commit 681292298ad97eebda56fa64f00c772c2c3c7e29
Author: law <law@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Mon Nov 9 19:56:57 2015 +0000
Re: [PATCH] Minor refactoring in tree-ssanames.c & freelists verifier
* tree-ssanames.c (verify_ssaname_freelists): Simplify check for
being in gimple/ssa form. Remove redundant check for SSA_NAME.
Fix comment typo.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@230049 138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 7911804..43a8d49 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2015-11-09 Jeff Law <law@redhat.com>
+
+ * tree-ssanames.c (verify_ssaname_freelists): Simplify check for
+ being in gimple/ssa form. Remove redundant check for SSA_NAME.
+ Fix comment typo.
+
2015-11-09 Michael Meissner <meissner@linux.vnet.ibm.com>
* config/rs6000/rs6000.opt (-mpower9-fusion): Add new switches for
diff --git a/gcc/tree-ssanames.c b/gcc/tree-ssanames.c
index 096b75b..b599bb5 100644
--- a/gcc/tree-ssanames.c
+++ b/gcc/tree-ssanames.c
@@ -124,17 +124,13 @@ ssanames_print_statistics (void)
DEBUG_FUNCTION void
verify_ssaname_freelists (struct function *fun)
{
- /* Do nothing if we are in RTL format. */
- basic_block bb;
- FOR_EACH_BB_FN (bb, fun)
- {
- if (bb->flags & BB_RTL)
- return;
- }
+ if (!gimple_in_ssa_p (fun))
+ return;
bitmap names_in_il = BITMAP_ALLOC (NULL);
/* Walk the entire IL noting every SSA_NAME we see. */
+ basic_block bb;
FOR_EACH_BB_FN (bb, fun)
{
tree t;
@@ -163,8 +159,7 @@ verify_ssaname_freelists (struct function *fun)
ssa_op_iter iter;
gimple *stmt = gsi_stmt (gsi);
FOR_EACH_SSA_TREE_OPERAND (t, stmt, iter, SSA_OP_ALL_OPERANDS)
- if (TREE_CODE (t) == SSA_NAME)
- bitmap_set_bit (names_in_il, SSA_NAME_VERSION (t));
+ bitmap_set_bit (names_in_il, SSA_NAME_VERSION (t));
}
}
@@ -218,7 +213,7 @@ verify_ssaname_freelists (struct function *fun)
debug/non-debug compilations have the same SSA_NAMEs. So for each
lost SSA_NAME, see if it's likely one from that wart. These will always
be marked as default definitions. So we loosely assume that anything
- marked as a default definition isn't leaked by pretening they are
+ marked as a default definition isn't leaked by pretending they are
in the IL. */
for (unsigned int i = UNUSED_NAME_VERSION + 1; i < num_ssa_names; i++)
if (ssa_name (i) && SSA_NAME_IS_DEFAULT_DEF (ssa_name (i)))
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-11-09 19:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-09 9:02 [PATCH] Minor refactoring in tree-ssanames.c & freelists verifier Jeff Law
2015-11-09 15:00 ` Michael Matz
2015-11-09 19:59 ` Jeff Law
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).