Hello,
I am going to commit the following patch to fix PR 111688 (bootstrap on
ppc64le broken) and will re-fix 108007 (issues with IPA-SRA when user
explicitely turns off DCE) when I figure out what's going wrong.
Sorry for the breakage,
Martin
[PATCH] Revert "ipa: Self-DCE of uses of removed call LHSs (PR 108007)"
This reverts commit 1be18ea110a2d69570dbc494588a7c73173883be.
As reported in PR bootstrap/111688, it broke ppc64le bootstrap because
of a debug-compare failure.
---
gcc/cgraph.cc | 10 +---
gcc/cgraph.h | 9 +--
gcc/ipa-param-manipulation.cc | 88 ++++++++---------------------
gcc/ipa-param-manipulation.h | 3 +-
gcc/testsuite/gcc.dg/ipa/pr108007.c | 32 -----------
gcc/tree-inline.cc | 28 ++++-----
6 files changed, 38 insertions(+), 132 deletions(-)
delete mode 100644 gcc/testsuite/gcc.dg/ipa/pr108007.c
diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index b82367ac342..e41e5ad3ae7 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -1403,17 +1403,11 @@ cgraph_edge::redirect_callee (cgraph_node *n)
speculative indirect call, remove "speculative" of the indirect call and
also redirect stmt to it's final direct target.
- When called from within tree-inline, KILLED_SSAs has to contain the pointer
- to killed_new_ssa_names within the copy_body_data structure and SSAs
- discovered to be useless (if LHS is removed) will be added to it, otherwise
- it needs to be NULL.
-
It is up to caller to iteratively transform each "speculative"
direct call as appropriate. */
gimple *
-cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e,
- hash_set <tree> *killed_ssas)
+cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e)
{
tree decl = gimple_call_fndecl (e->call_stmt);
gcall *new_stmt;
@@ -1533,7 +1527,7 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e,
remove_stmt_from_eh_lp (e->call_stmt);
tree old_fntype = gimple_call_fntype (e->call_stmt);
- new_stmt = padjs->modify_call (e, false, killed_ssas);
+ new_stmt = padjs->modify_call (e, false);
cgraph_node *origin = e->callee;
while (origin->clone_of)
origin = origin->clone_of;
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index d7162efeeb4..cedaaac3a45 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1833,16 +1833,9 @@ public:
speculative indirect call, remove "speculative" of the indirect call and
also redirect stmt to it's final direct target.
- When called from within tree-inline, KILLED_SSAs has to contain the
- pointer to killed_new_ssa_names within the copy_body_data structure and
- SSAs discovered to be useless (if LHS is removed) will be added to it,
- otherwise it needs to be NULL.
-
It is up to caller to iteratively transform each "speculative"
direct call as appropriate. */
- static gimple *redirect_call_stmt_to_callee (cgraph_edge *e,
- hash_set <tree>
- *killed_ssas = nullptr);
+ static gimple *redirect_call_stmt_to_callee (cgraph_edge *e);
/* Create clone of edge in the node N represented
by CALL_EXPR the callgraph. */
diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc
index 014939bf754..ae52f17b2c9 100644
--- a/gcc/ipa-param-manipulation.cc
+++ b/gcc/ipa-param-manipulation.cc
@@ -593,66 +593,14 @@ isra_get_ref_base_and_offset (tree expr, tree *base_p,
unsigned *unit_offset_p)
return true;
}
-/* Remove all statements that use NAME and transitively those that use the
- result of such statements. KILLED_SSAS contains the SSA_NAMEs that are
- already being or have been processed and new ones need to be added to it.
- The funtction only has to process situations handled by
- ssa_name_only_returned_p in ipa-sra.cc with the exception that it can assume
- it must never reach a use in a return statement. */
-
-static void
-purge_transitive_uses (tree name, hash_set <tree> *killed_ssas)
-{
- imm_use_iterator imm_iter;
- gimple *stmt;
- auto_vec <tree, 4> worklist;
-
- worklist.safe_push (name);
- while (!worklist.is_empty ())
- {
- tree cur_name = worklist.pop ();
- FOR_EACH_IMM_USE_STMT (stmt, imm_iter, cur_name)
- {
- if (gimple_debug_bind_p (stmt))
- {
- /* When runing within tree-inline, we will never end up here but
- adding the SSAs to killed_ssas will do the trick in this case
- and the respective debug statements will get reset. */
- gimple_debug_bind_reset_value (stmt);
- update_stmt (stmt);
- continue;
- }
-
- tree lhs = NULL_TREE;
- if (is_gimple_assign (stmt))
- lhs = gimple_assign_lhs (stmt);
- else if (gimple_code (stmt) == GIMPLE_PHI)
- lhs = gimple_phi_result (stmt);
- gcc_assert (lhs
- && (TREE_CODE (lhs) == SSA_NAME)
- && !gimple_vdef (stmt));
- if (!killed_ssas->add (lhs))
- {
- worklist.safe_push (lhs);
- gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
- gsi_remove (&gsi, true);
- }
- }
- }
-}
-
/* Modify actual arguments of a function call in statement currently belonging
to CS, and make it call CS->callee->decl. Return the new statement that
replaced the old one. When invoked, cfun and current_function_decl have to
- be set to the caller. When called from within tree-inline, KILLED_SSAs has
- to contain the pointer to killed_new_ssa_names within the copy_body_data
- structure and SSAs discovered to be useless (if LHS is removed) will be
- added to it, otherwise it needs to be NULL. */
+ be set to the caller. */
gcall *
ipa_param_adjustments::modify_call (cgraph_edge *cs,
- bool update_references,
- hash_set <tree> *killed_ssas)
+ bool update_references)
{
gcall *stmt = cs->call_stmt;
tree callee_decl = cs->callee->decl;
@@ -962,20 +910,32 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs,
gcall *new_stmt = gimple_build_call_vec (callee_decl, vargs);
- hash_set <tree> *ssas_to_remove = NULL;
+ tree ssa_to_remove = NULL;
if (tree lhs = gimple_call_lhs (stmt))
{
if (!m_skip_return)
gimple_call_set_lhs (new_stmt, lhs);
else if (TREE_CODE (lhs) == SSA_NAME)
{
- if (!killed_ssas)
+ /* LHS should now by a default-def SSA. Unfortunately default-def
+ SSA_NAMEs need a backing variable (or at least some code examining
+ SSAs assumes it is non-NULL). So we either have to re-use the
+ decl we have at hand or introdice a new one. */
+ tree repl = create_tmp_var (TREE_TYPE (lhs), "removed_return");
+ repl = get_or_create_ssa_default_def (cfun, repl);
+ SSA_NAME_IS_DEFAULT_DEF (repl) = true;
+ imm_use_iterator ui;
+ use_operand_p use_p;
+ gimple *using_stmt;
+ FOR_EACH_IMM_USE_STMT (using_stmt, ui, lhs)
{
- ssas_to_remove = new hash_set<tree> (8);
- killed_ssas = ssas_to_remove;
+ FOR_EACH_IMM_USE_ON_STMT (use_p, ui)
+ {
+ SET_USE (use_p, repl);
+ }
+ update_stmt (using_stmt);
}
- killed_ssas->add (lhs);
- purge_transitive_uses (lhs, killed_ssas);
+ ssa_to_remove = lhs;
}
}
@@ -994,12 +954,8 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs,
fprintf (dump_file, "\n");
}
gsi_replace (&gsi, new_stmt, true);
- if (ssas_to_remove)
- {
- for (tree sn : *ssas_to_remove)
- release_ssa_name (sn);
- delete ssas_to_remove;
- }
+ if (ssa_to_remove)
+ release_ssa_name (ssa_to_remove);
if (update_references)
do
{
diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h
index b7e56fe6379..d6a26e9ad36 100644
--- a/gcc/ipa-param-manipulation.h
+++ b/gcc/ipa-param-manipulation.h
@@ -224,8 +224,7 @@ public:
/* Modify a call statement arguments (and possibly remove the return value)
as described in the data fields of this class. */
- gcall *modify_call (cgraph_edge *cs, bool update_references,
- hash_set <tree> *killed_ssas);
+ gcall *modify_call (cgraph_edge *cs, bool update_references);
/* Return if the first parameter is left intact. */
bool first_param_intact_p ();
/* Build a function type corresponding to the modified call. */
diff --git a/gcc/testsuite/gcc.dg/ipa/pr108007.c
b/gcc/testsuite/gcc.dg/ipa/pr108007.c
deleted file mode 100644
index 77fc95975cf..00000000000
--- a/gcc/testsuite/gcc.dg/ipa/pr108007.c
+++ /dev/null
@@ -1,32 +0,0 @@
-/* { dg-do run } */
-/* { dg-options "-Os -fno-dce -fno-tree-dce -g" } */
-
-/* This tests that when IPA-SRA removes a LHS of a call statement which, in the
- original source, is fed into a useless operation which however can trap when
- given nonsensical input, that we remove it even when the user has turned off
- normal DCE. */
-
-int a, b, d, e, f = 10000000, h;
-short c, g;
-static int *i() {
- g = f;
- L:
- h = e = ~g;
- g = ~f % g & e;
- if (!g)
- goto L;
- c++;
- while (g < 1)
- ;
- return &a;
-}
-static void k() {
- int *l, m = 2;
- l = i();
- for (; d < 1; d++)
- m |= *l >= b;
-}
-int main() {
- k();
- return 0;
-}
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index 8daef6955fd..d63060c9429 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -2988,19 +2988,20 @@ redirect_all_calls (copy_body_data * id, basic_block bb)
struct cgraph_edge *edge = id->dst_node->get_edge (stmt);
if (edge)
{
- if (!id->killed_new_ssa_names)
- id->killed_new_ssa_names = new hash_set<tree> (16);
gimple *new_stmt
- = cgraph_edge::redirect_call_stmt_to_callee (edge,
- id->killed_new_ssa_names);
+ = cgraph_edge::redirect_call_stmt_to_callee (edge);
+ /* If IPA-SRA transformation, run as part of edge redirection,
+ removed the LHS because it is unused, save it to
+ killed_new_ssa_names so that we can prune it from debug
+ statements. */
if (old_lhs
&& TREE_CODE (old_lhs) == SSA_NAME
&& !gimple_call_lhs (new_stmt))
- /* In case of IPA-SRA removing the LHS, the name should have
- been already added to the hash. But in case of redirecting
- to builtin_unreachable it was not and the name still should
- be pruned from debug statements. */
- id->killed_new_ssa_names->add (old_lhs);
+ {
+ if (!id->killed_new_ssa_names)
+ id->killed_new_ssa_names = new hash_set<tree> (16);
+ id->killed_new_ssa_names->add (old_lhs);
+ }
if (stmt == last && id->call_stmt && maybe_clean_eh_stmt (stmt))
gimple_purge_dead_eh_edges (bb);
@@ -3327,13 +3328,8 @@ copy_body (copy_body_data *id,
body = copy_cfg_body (id, entry_block_map, exit_block_map,
new_entry);
copy_debug_stmts (id);
- if (id->killed_new_ssa_names)
- {
- for (tree sn : *id->killed_new_ssa_names)
- release_ssa_name (sn);
- delete id->killed_new_ssa_names;
- id->killed_new_ssa_names = NULL;
- }
+ delete id->killed_new_ssa_names;
+ id->killed_new_ssa_names = NULL;
return body;
}
--
2.42.0
