]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
Revert "ipa: Self-DCE of uses of removed call LHSs (PR 108007)"
authorMartin Jambor <mjambor@suse.cz>
Thu, 5 Oct 2023 12:08:47 +0000 (14:08 +0200)
committerMartin Jambor <mjambor@suse.cz>
Thu, 5 Oct 2023 12:10:00 +0000 (14:10 +0200)
This reverts commit 1be18ea110a2d69570dbc494588a7c73173883be.

As reported in PR bootstrap/111688, it broke ppc64le bootstrap because
of a debug-compare failure.

gcc/cgraph.cc
gcc/cgraph.h
gcc/ipa-param-manipulation.cc
gcc/ipa-param-manipulation.h
gcc/testsuite/gcc.dg/ipa/pr108007.c [deleted file]
gcc/tree-inline.cc

index b82367ac342aa7295952dba8a9ec2c32dea916c5..e41e5ad3ae74d1fd73aef79f7cf96dac0dffa015 100644 (file)
@@ -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;
index d7162efeeb40242579d17175f1d9988a6cad775b..cedaaac3a45b774f87ae176e9b961df8c0e61466 100644 (file)
@@ -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.  */
index 014939bf7543a620fceb7ad530e947e0a3bcf719..ae52f17b2c9dcd23009450deb8b6a7d4d7edddd9 100644 (file)
@@ -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
       {
index b7e56fe6379d288e3201ed9e283e80327a93e98e..d6a26e9ad36f6f45acc9bfe53450ea3698fa3253 100644 (file)
@@ -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 (file)
index 77fc959..0000000
+++ /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;
-}
index 8daef6955fd7bdc9045dea39197717b247bb10ae..d63060c9429c456f769649afc075a4fec450678d 100644 (file)
@@ -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;
 }