stabilize store merging

Don't let pointer randomization change the order in which we process
store chains.  This may cause SSA_NAMEs to be released in different
order, and if they're reused later, they may cause differences in SSA
partitioning, leading to differences in expand, and ultimately to
different code.

bootstrap-debug-lean (-fcompare-debug) on i686-linux-gnu has failed in
haifa-sched.c since r245196 exposed the latent ordering problem in
store merging.  In this case, the IR differences (different SSA names
selected for copies in out-of-SSA, resulting in some off-by-one
differences in pseudos) were not significant enough to be visible in
the compiler output.


for  gcc/ChangeLog

	* gimple-ssa-store-merging.c (struct imm_store_chain_info):
	Add linked-list forward and backlinks.  Insert on
	construction, remove on destruction.
	(class pass_store_merging): Add m_stores_head field.
	(pass_store_merging::terminate_and_process_all_chains):
	Iterate over m_stores_head list.
	(pass_store_merging::terminate_all_aliasing_chains):
	Likewise.
	(pass_store_merging::execute): Check for debug stmts first.
	Push new chains onto the m_stores_head stack.

From-SVN: r246213
This commit is contained in:
Alexandre Oliva 2017-03-16 23:31:01 +00:00 committed by Alexandre Oliva
parent 2e92d7ada9
commit 50b6d6761b
2 changed files with 62 additions and 16 deletions

View File

@ -1,3 +1,16 @@
2017-03-16 Alexandre Oliva <aoliva@redhat.com>
* gimple-ssa-store-merging.c (struct imm_store_chain_info):
Add linked-list forward and backlinks. Insert on
construction, remove on destruction.
(class pass_store_merging): Add m_stores_head field.
(pass_store_merging::terminate_and_process_all_chains):
Iterate over m_stores_head list.
(pass_store_merging::terminate_all_aliasing_chains):
Likewise.
(pass_store_merging::execute): Check for debug stmts first.
Push new chains onto the m_stores_head stack.
2017-03-16 Michael Meissner <meissner@linux.vnet.ibm.com>
PR target/71294

View File

@ -675,11 +675,34 @@ merged_store_group::apply_stores ()
struct imm_store_chain_info
{
/* Doubly-linked list that imposes an order on chain processing.
PNXP (prev's next pointer) points to the head of a list, or to
the next field in the previous chain in the list.
See pass_store_merging::m_stores_head for more rationale. */
imm_store_chain_info *next, **pnxp;
tree base_addr;
auto_vec<struct store_immediate_info *> m_store_info;
auto_vec<merged_store_group *> m_merged_store_groups;
imm_store_chain_info (tree b_a) : base_addr (b_a) {}
imm_store_chain_info (imm_store_chain_info *&inspt, tree b_a)
: next (inspt), pnxp (&inspt), base_addr (b_a)
{
inspt = this;
if (next)
{
gcc_checking_assert (pnxp == next->pnxp);
next->pnxp = &next;
}
}
~imm_store_chain_info ()
{
*pnxp = next;
if (next)
{
gcc_checking_assert (&next == next->pnxp);
next->pnxp = pnxp;
}
}
bool terminate_and_process_chain ();
bool coalesce_immediate_stores ();
bool output_merged_store (merged_store_group *);
@ -718,6 +741,17 @@ public:
private:
hash_map<tree_operand_hash, struct imm_store_chain_info *> m_stores;
/* Form a doubly-linked stack of the elements of m_stores, so that
we can iterate over them in a predictable way. Using this order
avoids extraneous differences in the compiler output just because
of tree pointer variations (e.g. different chains end up in
different positions of m_stores, so they are handled in different
orders, so they allocate or release SSA names in different
orders, and when they get reused, subsequent passes end up
getting different SSA names, which may ultimately change
decisions when going out of SSA). */
imm_store_chain_info *m_stores_head;
bool terminate_and_process_all_chains ();
bool terminate_all_aliasing_chains (imm_store_chain_info **,
bool, gimple *);
@ -730,11 +764,11 @@ private:
bool
pass_store_merging::terminate_and_process_all_chains ()
{
hash_map<tree_operand_hash, struct imm_store_chain_info *>::iterator iter
= m_stores.begin ();
bool ret = false;
for (; iter != m_stores.end (); ++iter)
ret |= terminate_and_release_chain ((*iter).second);
while (m_stores_head)
ret |= terminate_and_release_chain (m_stores_head);
gcc_assert (m_stores.elements () == 0);
gcc_assert (m_stores_head == NULL);
return ret;
}
@ -799,15 +833,14 @@ pass_store_merging::terminate_all_aliasing_chains (imm_store_chain_info
}
}
hash_map<tree_operand_hash, struct imm_store_chain_info *>::iterator iter
= m_stores.begin ();
/* Check for aliasing with all other store chains. */
for (; iter != m_stores.end (); ++iter)
for (imm_store_chain_info *next = m_stores_head, *cur = next; cur; cur = next)
{
next = cur->next;
/* We already checked all the stores in chain_info and terminated the
chain if necessary. Skip it here. */
if (chain_info && (*chain_info) == (*iter).second)
if (chain_info && (*chain_info) == cur)
continue;
/* We can't use the base object here as that does not reliably exist.
@ -815,11 +848,11 @@ pass_store_merging::terminate_all_aliasing_chains (imm_store_chain_info
minimum and maximum offset and the maximum size we could improve
things here). */
ao_ref chain_ref;
ao_ref_init_from_ptr_and_size (&chain_ref, (*iter).first, NULL_TREE);
ao_ref_init_from_ptr_and_size (&chain_ref, cur->base_addr, NULL_TREE);
if (ref_maybe_used_by_stmt_p (stmt, &chain_ref)
|| stmt_may_clobber_ref_p_1 (stmt, &chain_ref))
{
terminate_and_release_chain ((*iter).second);
terminate_and_release_chain (cur);
ret = true;
}
}
@ -1336,6 +1369,9 @@ pass_store_merging::execute (function *fun)
{
gimple *stmt = gsi_stmt (gsi);
if (is_gimple_debug (stmt))
continue;
if (gimple_has_volatile_ops (stmt))
{
/* Terminate all chains. */
@ -1346,9 +1382,6 @@ pass_store_merging::execute (function *fun)
continue;
}
if (is_gimple_debug (stmt))
continue;
if (gimple_assign_single_p (stmt) && gimple_vdef (stmt)
&& !stmt_can_throw_internal (stmt)
&& lhs_valid_for_store_merging_p (gimple_assign_lhs (stmt)))
@ -1456,7 +1489,7 @@ pass_store_merging::execute (function *fun)
terminate_all_aliasing_chains (chain_info, false, stmt);
/* Start a new chain. */
struct imm_store_chain_info *new_chain
= new imm_store_chain_info (base_addr);
= new imm_store_chain_info (m_stores_head, base_addr);
info = new store_immediate_info (bitsize, bitpos,
stmt, 0);
new_chain->m_store_info.safe_push (info);