Hi,
back in January in
http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00848.html Eric pointed
out a testcase where the problem was SRA not scalarizing an aggregate
because it was involved in a throwing statement. The reason is that
SRA is likely to need to append new statements after each one where a
replaced aggregate is present, but throwing statements must end their
BBs. This patch comes up with a fix for most such situations by
adding these new statements onto a single successor non-EH edge, if
there is one and only one such edge.
I have bootstrapped and tested a very similar version on x86_64-linux,
bootstrap and testing of this exact one is currently underway. OK for
trunk? Eric, if and once this gets in, can you please add the
testcase from your original post to the suite?
Thanks,
Martin
2014-04-15 Martin Jambor <mjam...@suse.cz>
* tree-sra.c (single_non_eh_succ): New function.
(disqualify_ops_if_throwing_stmt): Renamed to
disqualify_if_bad_bb_terminating_stmt. Allow throwing statements
having one non-EH successor BB.
(gsi_for_eh_followups): New function.
(sra_modify_expr): If stmt ends bb, use single non-EH successor to
generate loads into replacements.
(sra_modify_assign): Likewise and and also use the simple path for
such statements.
(sra_modify_function_body): Iterate safely over BBs.
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index ffef13d..4fd0f5e 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -1142,17 +1142,41 @@ build_access_from_expr (tree expr, gimple stmt, bool
write)
return false;
}
-/* Disqualify LHS and RHS for scalarization if STMT must end its basic block in
- modes in which it matters, return true iff they have been disqualified. RHS
- may be NULL, in that case ignore it. If we scalarize an aggregate in
- intra-SRA we may need to add statements after each statement. This is not
- possible if a statement unconditionally has to end the basic block. */
+/* Return the single non-EH successor edge of BB or NULL if there is none or
+ more than one. */
+
+static edge
+single_non_eh_succ (basic_block bb)
+{
+ edge e, res = NULL;
+ edge_iterator ei;
+
+ FOR_EACH_EDGE (e, ei, bb->succs)
+ if (!(e->flags & EDGE_EH))
+ {
+ if (res)
+ return NULL;
+ res = e;
+ }
+
+ return res;
+}
+
+/* Disqualify LHS and RHS for scalarization if STMT has to terminate its BB and
+ there is no alternative spot where to put statements SRA might need to
+ generate after it. The spot we are looking for is an edge leading to a
+ single non-EH successor, if it exists and is indeed single. RHS may be
+ NULL, in that case ignore it. */
+
static bool
-disqualify_ops_if_throwing_stmt (gimple stmt, tree lhs, tree rhs)
+disqualify_if_bad_bb_terminating_stmt (gimple stmt, tree lhs, tree rhs)
{
if ((sra_mode == SRA_MODE_EARLY_INTRA || sra_mode == SRA_MODE_INTRA)
- && (stmt_can_throw_internal (stmt) || stmt_ends_bb_p (stmt)))
+ && stmt_ends_bb_p (stmt))
{
+ if (single_non_eh_succ (gimple_bb (stmt)))
+ return false;
+
disqualify_base_of_expr (lhs, "LHS of a throwing stmt.");
if (rhs)
disqualify_base_of_expr (rhs, "RHS of a throwing stmt.");
@@ -1180,7 +1204,7 @@ build_accesses_from_assign (gimple stmt)
lhs = gimple_assign_lhs (stmt);
rhs = gimple_assign_rhs1 (stmt);
- if (disqualify_ops_if_throwing_stmt (stmt, lhs, rhs))
+ if (disqualify_if_bad_bb_terminating_stmt (stmt, lhs, rhs))
return false;
racc = build_access_from_expr_1 (rhs, stmt, false);
@@ -1319,7 +1343,7 @@ scan_function (void)
}
t = gimple_call_lhs (stmt);
- if (t && !disqualify_ops_if_throwing_stmt (stmt, t, NULL))
+ if (t && !disqualify_if_bad_bb_terminating_stmt (stmt, t, NULL))
ret |= build_access_from_expr (t, stmt, true);
break;
@@ -2734,6 +2758,19 @@ get_access_for_expr (tree expr)
return get_var_base_offset_size_access (base, offset, max_size);
}
+/* Split the single non-EH successor edge from BB (there must be exactly one)
+ and return a gimple iterator to the new block. */
+
+static gimple_stmt_iterator
+gsi_for_eh_followups (basic_block bb)
+{
+ edge e = single_non_eh_succ (bb);
+ gcc_assert (e);
+
+ basic_block new_bb = split_edge (e);
+ return gsi_start_bb (new_bb);
+}
+
/* Replace the expression EXPR with a scalar replacement if there is one and
generate other statements to do type conversion or subtree copying if
necessary. GSI is used to place newly created statements, WRITE is true if
@@ -2763,6 +2800,13 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi,
bool write)
type = TREE_TYPE (*expr);
loc = gimple_location (gsi_stmt (*gsi));
+ gimple_stmt_iterator alt_gsi = gsi_none ();
+ if (write && stmt_ends_bb_p (gsi_stmt (*gsi)))
+ {
+ alt_gsi = gsi_for_eh_followups (gsi_bb (*gsi));
+ gsi = &alt_gsi;
+ }
+
if (access->grp_to_be_replaced)
{
tree repl = get_access_replacement (access);
@@ -3226,14 +3270,24 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator
*gsi)
if (modify_this_stmt
|| gimple_has_volatile_ops (*stmt)
|| contains_vce_or_bfcref_p (rhs)
- || contains_vce_or_bfcref_p (lhs))
+ || contains_vce_or_bfcref_p (lhs)
+ || stmt_can_throw_internal (*stmt)
+ || stmt_ends_bb_p (*stmt))
{
if (access_has_children_p (racc))
generate_subtree_copies (racc->first_child, racc->base, 0, 0, 0,
gsi, false, false, loc);
if (access_has_children_p (lacc))
- generate_subtree_copies (lacc->first_child, lacc->base, 0, 0, 0,
- gsi, true, true, loc);
+ {
+ gimple_stmt_iterator alt_gsi = gsi_none ();
+ if (stmt_ends_bb_p (*stmt))
+ {
+ alt_gsi = gsi_for_eh_followups (gsi_bb (*gsi));
+ gsi = &alt_gsi;
+ }
+ generate_subtree_copies (lacc->first_child, lacc->base, 0, 0, 0,
+ gsi, true, true, loc);
+ }
sra_stats.separate_lhs_rhs_handling++;
/* This gimplification must be done after generate_subtree_copies,
@@ -3327,8 +3381,13 @@ sra_modify_function_body (void)
{
bool cfg_changed = false;
basic_block bb;
+ vec<basic_block> bb_list = vNULL;
FOR_EACH_BB_FN (bb, cfun)
+ bb_list.safe_push (bb);
+
+ unsigned bb_vec_index;
+ FOR_EACH_VEC_ELT (bb_list, bb_vec_index, bb)
{
gimple_stmt_iterator gsi = gsi_start_bb (bb);
while (!gsi_end_p (gsi))
@@ -3397,6 +3456,7 @@ sra_modify_function_body (void)
}
}
+ bb_list.release ();
return cfg_changed;
}
