diff mbox series

Revert "ipa: Self-DCE of uses of removed call LHSs (PR 108007)"

Message ID ri6edi9i8jw.fsf@suse.cz
State New
Headers show
Series Revert "ipa: Self-DCE of uses of removed call LHSs (PR 108007)" | expand

Commit Message

Martin Jambor Oct. 5, 2023, 11:59 a.m. UTC
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 mbox series

Patch

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;
 }