ipa-sra: Don't consider CLOBBERS as writes preventing splitting

When IPA-SRA detects whether a parameter passed by reference is
written to, it does not special case CLOBBERs which means it often
bails out unnecessarily, especially when dealing with C++ destructors.
Fixed by the obvious continue in the two relevant loops and by adding
a simple function that marks the clobbers in the transformation code
as statements to be removed.

gcc/ChangeLog:

2023-08-04  Martin Jambor  <mjambor@suse.cz>

	PR ipa/110378
	* ipa-param-manipulation.h (class ipa_param_body_adjustments): New
	members get_ddef_if_exists_and_is_used and mark_clobbers_dead.
	* ipa-sra.cc (isra_track_scalar_value_uses): Ignore clobbers.
	(ptr_parm_has_nonarg_uses): Likewise.
	* ipa-param-manipulation.cc
	(ipa_param_body_adjustments::get_ddef_if_exists_and_is_used): New.
	(ipa_param_body_adjustments::mark_dead_statements): Move initial
	checks to get_ddef_if_exists_and_is_used.
	(ipa_param_body_adjustments::mark_clobbers_dead): New.
	(ipa_param_body_adjustments::common_initialization): Call
	mark_clobbers_dead when splitting.

gcc/testsuite/ChangeLog:

2023-07-31  Martin Jambor  <mjambor@suse.cz>

	PR ipa/110378
	* g++.dg/ipa/pr110378-1.C: New test.

diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc
index a286af7f5d92d3f2a2f1fbf82784c4ef10860c25..4a185ddbdf4084d3eefb1d8103c8b4e6630ef73d 100644 (file)
--- a/gcc/ipa-param-manipulation.cc
+++ b/gcc/ipa-param-manipulation.cc
@@ -1072,6 +1072,20 @@ ipa_param_body_adjustments::carry_over_param (tree t)
   return new_parm;
 }
 
+/* If DECL is a gimple register that has a default definition SSA name and that
+   has some uses, return the default definition, otherwise return NULL_TREE.  */
+
+tree
+ipa_param_body_adjustments::get_ddef_if_exists_and_is_used (tree decl)
+{
+ if (!is_gimple_reg (decl))
+    return NULL_TREE;
+  tree ddef = ssa_default_def (m_id->src_cfun, decl);
+  if (!ddef || has_zero_uses (ddef))
+    return NULL_TREE;
+  return ddef;
+}
+
 /* Populate m_dead_stmts given that DEAD_PARAM is going to be removed without
    any replacement or splitting.  REPL is the replacement VAR_SECL to base any
    remaining uses of a removed parameter on.  Push all removed SSA names that
@@ -1084,10 +1098,8 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param,
   /* Current IPA analyses which remove unused parameters never remove a
      non-gimple register ones which have any use except as parameters in other
      calls, so we can safely leve them as they are.  */
-  if (!is_gimple_reg (dead_param))
-    return;
-  tree parm_ddef = ssa_default_def (m_id->src_cfun, dead_param);
-  if (!parm_ddef || has_zero_uses (parm_ddef))
+  tree parm_ddef = get_ddef_if_exists_and_is_used (dead_param);
+  if (!parm_ddef)
     return;
 
   auto_vec<tree, 4> stack;
@@ -1169,6 +1181,28 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param,
   m_dead_ssa_debug_equiv.put (parm_ddef, dp_ddecl);
 }
 
+/* Put all clobbers of of dereference of default definition of PARAM into
+   m_dead_stmts.  */
+
+void
+ipa_param_body_adjustments::mark_clobbers_dead (tree param)
+{
+  if (!is_gimple_reg (param))
+    return;
+  tree ddef = get_ddef_if_exists_and_is_used (param);
+  if (!ddef)
+    return;
+
+ imm_use_iterator imm_iter;
+ use_operand_p use_p;
+ FOR_EACH_IMM_USE_FAST (use_p, imm_iter, ddef)
+   {
+     gimple *stmt = USE_STMT (use_p);
+     if (gimple_clobber_p (stmt))
+       m_dead_stmts.add (stmt);
+   }
+}
+
 /* Callback to walk_tree.  If REMAP is an SSA_NAME that is present in hash_map
    passed in DATA, replace it with unshared version of what it was mapped to.
    If an SSA argument would be remapped to NULL, the whole operation needs to
@@ -1504,6 +1538,8 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl,
               that will guide what not to copy to the new body.  */
            if (!split[i])
              mark_dead_statements (m_oparms[i], &ssas_to_process_debug);
+           else
+             mark_clobbers_dead (m_oparms[i]);
            if (MAY_HAVE_DEBUG_STMTS
                && is_gimple_reg (m_oparms[i]))
              m_reset_debug_decls.safe_push (m_oparms[i]);

diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h
index 9798eedf0b63c07e63d020595caed6d69225e089..d6a26e9ad36f6f45acc9bfe53450ea3698fa3253 100644 (file)
--- a/gcc/ipa-param-manipulation.h
+++ b/gcc/ipa-param-manipulation.h
@@ -378,7 +378,9 @@ private:
   bool modify_call_stmt (gcall **stmt_p, gimple *orig_stmt);
   bool modify_cfun_body ();
   void reset_debug_stmts ();
+  tree get_ddef_if_exists_and_is_used (tree decl);
   void mark_dead_statements (tree dead_param, vec<tree> *debugstack);
+  void mark_clobbers_dead (tree dead_param);
   bool prepare_debug_expressions (tree dead_ssa);
 
   /* Declaration of the function that is being transformed.  */

diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
index c35e03b7abdcaa28cb1c51b37a50f008695f27ba..edba364f56ef7cdc61db829394698ecb19f6c4e9 100644 (file)
--- a/gcc/ipa-sra.cc
+++ b/gcc/ipa-sra.cc
@@ -898,7 +898,8 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name,
 
   FOR_EACH_IMM_USE_STMT (stmt, imm_iter, name)
     {
-      if (is_gimple_debug (stmt))
+      if (is_gimple_debug (stmt)
+         || gimple_clobber_p (stmt))
        continue;
 
       /* TODO: We could handle at least const builtin functions like arithmetic
@@ -1056,7 +1057,8 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm,
       unsigned uses_ok = 0;
       use_operand_p use_p;
 
-      if (is_gimple_debug (stmt))
+      if (is_gimple_debug (stmt)
+         || gimple_clobber_p (stmt))
        continue;
 
       if (gimple_assign_single_p (stmt))

diff --git a/gcc/testsuite/g++.dg/ipa/pr110378-1.C b/gcc/testsuite/g++.dg/ipa/pr110378-1.C
new file mode 100644 (file)
index 0000000..fda7699
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr110378-1.C
@@ -0,0 +1,48 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-sra -fdump-tree-optimized-slim"  } */
+
+/* Test that even though destructors end with clobbering all of *this, it
+   should not prevent IPA-SRA.  */
+
+namespace {
+
+  class foo
+  {
+  public:
+    short move_offset_of_a;
+    int *a;
+    foo(int c)
+    {
+      a = new int[c];
+      a[0] = 4;
+    }
+    __attribute__((noinline)) ~foo();
+    int f ()
+    {
+      return a[0] + 1;
+    }
+  };
+
+  volatile int v1 = 4;
+
+  __attribute__((noinline)) foo::~foo()
+  {
+    delete[] a;
+    return;
+  }
+
+
+}
+
+volatile int v2 = 20;
+
+int test (void)
+{
+  foo shouldnotexist(v2);
+  v2 = shouldnotexist.f();
+  return 0;
+}
+
+
+/* { dg-final { scan-ipa-dump "Will split parameter 0" "sra"  } } */
+/* { dg-final { scan-tree-dump-not "shouldnotexist" "optimized" } } */