public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-1848] ipa-sra: Introduce a mini-DCE to tree-inline.c (PR 93385)
@ 2021-06-28 16:28 Martin Jambor
  0 siblings, 0 replies; only message in thread
From: Martin Jambor @ 2021-06-28 16:28 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:2902991a6b61d473f7cb996a2b80eef4a90f8eda

commit r12-1848-g2902991a6b61d473f7cb996a2b80eef4a90f8eda
Author: Martin Jambor <mjambor@suse.cz>
Date:   Mon Jun 28 18:20:00 2021 +0200

    ipa-sra: Introduce a mini-DCE to tree-inline.c (PR 93385)
    
    I was asked by Richi to split my fix for PR 93385 for easier review
    into IPA-SRA materialization refactoring and the actual DCE addition.
    This is the second part that actually contains the DCE of statements
    that IPA-SRA should not leave behind because they can have problematic
    side effects, even if they are useless, so that we do not depend on
    tree-dce to remove them for correctness.
    
    The patch fixes the problem by doing a def-use walk when materializing
    clones, marking which statements should not be copied and which
    SSA_NAMEs do not need to be computed because eventually they would be
    DCEd.  We do this on the original function body and tree-inline simply
    does not copy statements which are "dead."
    
    The only complication is removing dead argument calls because that
    needs to be communicated to callee redirection code using the
    infrastructure introduced by the previous patch.
    
    I added all testcases of the original patch to this one, although some
    probably test behavior introduced in the previous patch.
    
    gcc/ChangeLog:
    
    2021-05-12  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/93385
            * ipa-param-manipulation.h (class ipa_param_body_adjustments): New
            members m_dead_stmts and m_dead_ssas.
            * ipa-param-manipulation.c
            (ipa_param_body_adjustments::mark_dead_statements): New function.
            (ipa_param_body_adjustments::common_initialization): Call it on
            all removed but not split parameters.
            (ipa_param_body_adjustments::ipa_param_body_adjustments): Initialize
            new mwmbers.
            (ipa_param_body_adjustments::modify_call_stmt): Remove arguments that
            are dead.
            * tree-inline.c (remap_gimple_stmt): Do not copy dead statements, reset
            dead debug statements.
            (copy_phis_for_bb): Do not copy dead PHI nodes.
    
    gcc/testsuite/ChangeLog:
    
    2021-03-22  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/93385
            * gcc.dg/ipa/pr93385.c: New test.
            * gcc.dg/ipa/ipa-sra-23.c: Likewise.
            * gcc.dg/ipa/ipa-sra-24.c: Likewise.
            * g++.dg/ipa/ipa-sra-4.C: Likewise.

Diff:
---
 gcc/ipa-param-manipulation.c          | 129 ++++++++++++++++++++++++++++++----
 gcc/ipa-param-manipulation.h          |   6 ++
 gcc/testsuite/g++.dg/ipa/ipa-sra-4.C  |  37 ++++++++++
 gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c |  24 +++++++
 gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c |  20 ++++++
 gcc/testsuite/gcc.dg/ipa/pr93385.c    |  27 +++++++
 gcc/tree-inline.c                     |  18 ++++-
 7 files changed, 243 insertions(+), 18 deletions(-)

diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
index 6a423391d2f..26b02d7aa95 100644
--- a/gcc/ipa-param-manipulation.c
+++ b/gcc/ipa-param-manipulation.c
@@ -970,6 +970,84 @@ ipa_param_body_adjustments::carry_over_param (tree t)
   return new_parm;
 }
 
+/* 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.  */
+
+void
+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))
+    return;
+
+  auto_vec<tree, 4> stack;
+  m_dead_ssas.add (parm_ddef);
+  stack.safe_push (parm_ddef);
+  while (!stack.is_empty ())
+    {
+      imm_use_iterator imm_iter;
+      use_operand_p use_p;
+      tree t = stack.pop ();
+
+      insert_decl_map (m_id, t, error_mark_node);
+      FOR_EACH_IMM_USE_FAST (use_p, imm_iter, t)
+	{
+	  gimple *stmt = USE_STMT (use_p);
+
+	  /* Calls containing dead arguments cannot be deleted,
+	     modify_call_stmt will instead remove just the argument later on.
+	     If isra_track_scalar_value_uses in ipa-sra.c is extended to look
+	     through const functions, we will need to do so here too.  */
+	  if (is_gimple_call (stmt)
+	      || (m_id->blocks_to_copy
+		  && !bitmap_bit_p (m_id->blocks_to_copy,
+				    gimple_bb (stmt)->index)))
+	    continue;
+
+	  if (is_gimple_debug (stmt))
+	    {
+	      m_dead_stmts.add (stmt);
+	      gcc_assert (gimple_debug_bind_p (stmt));
+	    }
+	  else if (gimple_code (stmt) == GIMPLE_PHI)
+	    {
+	      gphi *phi = as_a <gphi *> (stmt);
+	      int ix = PHI_ARG_INDEX_FROM_USE (use_p);
+
+	      if (!m_id->blocks_to_copy
+		  || bitmap_bit_p (m_id->blocks_to_copy,
+				   gimple_phi_arg_edge (phi, ix)->src->index))
+		{
+		  m_dead_stmts.add (phi);
+		  tree res = gimple_phi_result (phi);
+		  if (!m_dead_ssas.add (res))
+		    stack.safe_push (res);
+		}
+	    }
+	  else if (is_gimple_assign (stmt))
+	    {
+	      m_dead_stmts.add (stmt);
+	      if (!gimple_clobber_p (stmt))
+		{
+		  tree lhs = gimple_assign_lhs (stmt);
+		  gcc_assert (TREE_CODE (lhs) == SSA_NAME);
+		  if (!m_dead_ssas.add (lhs))
+		    stack.safe_push (lhs);
+		}
+	    }
+	  else
+	    /* IPA-SRA does not analyze other types of statements.  */
+	    gcc_unreachable ();
+	}
+    }
+}
+
 /* Common initialization performed by all ipa_param_body_adjustments
    constructors.  OLD_FNDECL is the declaration we take original arguments
    from, (it may be the same as M_FNDECL).  VARS, if non-NULL, is a pointer to
@@ -1003,6 +1081,9 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl,
   auto_vec<bool, 16> kept;
   kept.reserve_exact (m_oparms.length ());
   kept.quick_grow_cleared (m_oparms.length ());
+  auto_vec<bool, 16> split;
+  split.reserve_exact (m_oparms.length ());
+  split.quick_grow_cleared (m_oparms.length ());
 
   unsigned adj_len = vec_safe_length (m_adj_params);
   m_method2func = ((TREE_CODE (TREE_TYPE (m_fndecl)) == METHOD_TYPE)
@@ -1048,6 +1129,7 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl,
 	  if (apm->op == IPA_PARAM_OP_SPLIT)
 	    {
 	      m_split_modifications_p = true;
+	      split[prev_index] = true;
 	      register_replacement (apm, new_parm);
 	    }
         }
@@ -1080,6 +1162,11 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl,
 		/* Declare this new variable.  */
 		DECL_CHAIN (var) = *vars;
 		*vars = var;
+
+		/* If this is not a split but a real removal, init hash sets
+		   that will guide what not to copy to the new body.  */
+		if (!split[i])
+		  mark_dead_statements (m_oparms[i]);
 	      }
 	  }
 	else
@@ -1136,9 +1223,10 @@ ipa_param_body_adjustments
 ::ipa_param_body_adjustments (vec<ipa_adjusted_param, va_gc> *adj_params,
 			      tree fndecl)
   : m_adj_params (adj_params), m_adjustments (NULL), m_reset_debug_decls (),
-    m_split_modifications_p (false), m_fndecl (fndecl), m_id (NULL),
-    m_oparms (), m_new_decls (), m_new_types (), m_replacements (),
-    m_removed_decls (), m_removed_map (), m_method2func (false)
+    m_split_modifications_p (false), m_dead_stmts (), m_dead_ssas (),
+    m_fndecl (fndecl), m_id (NULL), m_oparms (), m_new_decls (),
+    m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (),
+    m_method2func (false)
 {
   common_initialization (fndecl, NULL, NULL);
 }
@@ -1152,9 +1240,9 @@ ipa_param_body_adjustments
 ::ipa_param_body_adjustments (ipa_param_adjustments *adjustments,
 			      tree fndecl)
   : m_adj_params (adjustments->m_adj_params), m_adjustments (adjustments),
-    m_reset_debug_decls (), m_split_modifications_p (false), m_fndecl (fndecl),
-    m_id (NULL), m_oparms (), m_new_decls (), m_new_types (),
-    m_replacements (), m_removed_decls (), m_removed_map (),
+    m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (),
+    m_dead_ssas (), m_fndecl (fndecl), m_id (NULL), m_oparms (), m_new_decls (),
+    m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (),
     m_method2func (false)
 {
   common_initialization (fndecl, NULL, NULL);
@@ -1175,9 +1263,10 @@ ipa_param_body_adjustments
 			      copy_body_data *id, tree *vars,
 			      vec<ipa_replace_map *, va_gc> *tree_map)
   : m_adj_params (adjustments->m_adj_params), m_adjustments (adjustments),
-    m_reset_debug_decls (), m_split_modifications_p (false), m_fndecl (fndecl),
-    m_id (id), m_oparms (), m_new_decls (), m_new_types (), m_replacements (),
-    m_removed_decls (), m_removed_map (), m_method2func (false)
+    m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (),
+    m_dead_ssas (),m_fndecl (fndecl), m_id (id), m_oparms (), m_new_decls (),
+    m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (),
+    m_method2func (false)
 {
   common_initialization (old_fndecl, vars, tree_map);
 }
@@ -1624,8 +1713,9 @@ ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p,
 		  && TREE_CODE (t) != IMAGPART_EXPR
 		  && TREE_CODE (t) != REALPART_EXPR);
 
-      /* The follow-up patch will check whether t needs to be removed, that's
-	 why this condition is in the loop.  */
+      if (TREE_CODE (t) == SSA_NAME
+	  && m_dead_ssas.contains (t))
+	recreate = true;
 
       if (!m_split_modifications_p)
 	continue;
@@ -1763,10 +1853,19 @@ ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p,
       else
 	{
 	  tree t = gimple_call_arg (stmt, i);
-	  modify_expression (&t, true);
-	  vargs.safe_push (t);
-	  index_map.safe_push (new_arg_idx);
-	  new_arg_idx++;
+	  if (TREE_CODE (t) == SSA_NAME
+	      && m_dead_ssas.contains (t))
+	    {
+	      always_copy_delta--;
+	      index_map.safe_push (-1);
+	    }
+	  else
+	    {
+	      modify_expression (&t, true);
+	      vargs.safe_push (t);
+	      index_map.safe_push (new_arg_idx);
+	      new_arg_idx++;
+	    }
 	}
     }
 
diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h
index 31dcc4b9768..afcbc09bf33 100644
--- a/gcc/ipa-param-manipulation.h
+++ b/gcc/ipa-param-manipulation.h
@@ -342,6 +342,12 @@ public:
   /* Set to true if there are any IPA_PARAM_OP_SPLIT adjustments among stored
      adjustments.  */
   bool m_split_modifications_p;
+
+  /* Sets of statements and SSA_NAMEs that only manipulate data from parameters
+     removed because they are not necessary.  */
+  hash_set<gimple *> m_dead_stmts;
+  hash_set<tree> m_dead_ssas;
+
 private:
   void common_initialization (tree old_fndecl, tree *vars,
 			      vec<ipa_replace_map *, va_gc> *tree_map);
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-sra-4.C b/gcc/testsuite/g++.dg/ipa/ipa-sra-4.C
new file mode 100644
index 00000000000..56d59f9fd9a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-sra-4.C
@@ -0,0 +1,37 @@
+/* { dg-do compile { target c++11 } } */
+/* { dg-options "-O2 -fipa-sra"  } */
+
+void __throw_bad_alloc() __attribute__((__noreturn__));
+void __throw_bad_array_new_length();
+template <typename> class allocator {};
+template <typename> struct allocator_traits;
+int *allocate___trans_tmp_2;
+template <typename _Tp> struct allocator_traits<allocator<_Tp>> {
+  using allocator_type = allocator<_Tp>;
+  using pointer = _Tp *;
+  using size_type = long;
+  static pointer allocate(allocator_type &, size_type __n) {
+    long __trans_tmp_3 = __n;
+    if (__builtin_expect(__trans_tmp_3, false))
+      if (__trans_tmp_3)
+        __throw_bad_array_new_length();
+    operator new(sizeof(int));
+    return allocate___trans_tmp_2;
+  }
+};
+class throw_allocator_base {
+  allocator<int> _M_allocator;
+public:
+  int *allocate(long __n) {
+    if (__n)
+      __throw_bad_alloc();
+    int *a = allocator_traits<allocator<int>>::allocate(_M_allocator, __n);
+    return a;
+  }
+};
+template <typename Alloc> void check_allocate_max_size() {
+  Alloc a;
+  long __trans_tmp_1 = 0;
+  a.allocate(__trans_tmp_1 + 1);
+}
+int main() { check_allocate_max_size<throw_allocator_base>(); }
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c
new file mode 100644
index 00000000000..f438b509614
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-O2"  } */
+
+extern int g;
+
+static int __attribute__((noinline))
+bar (int i, int j)
+{
+  return 2*g + i;
+}
+
+static int __attribute__((noinline))
+foo (int i, int j)
+{
+  if (i > 5)
+    j = 22;
+  return bar (i, j) + 1;
+}
+
+int
+entry (int l, int k)
+{
+  return foo (l, k);
+}
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c
new file mode 100644
index 00000000000..7b5bf0825fc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wmaybe-uninitialized -Werror"  } */
+
+int *ttmp_1;
+_Bool pt_ins_tipdo, pq_ins_apd, pq_ins_tt2;
+int gtrphdt;
+
+void pl_ins(int, _Bool, _Bool);
+inline void pt_ins(int *, _Bool apdo) {
+  int list = *ttmp_1;
+  pl_ins(list, apdo, pt_ins_tipdo);
+}
+void pq_ins(int *t) {
+  if (pq_ins_tt2)
+    pt_ins(t, pq_ins_apd);
+}
+int gtr_post_hd() {
+  pq_ins(&gtrphdt);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/ipa/pr93385.c b/gcc/testsuite/gcc.dg/ipa/pr93385.c
new file mode 100644
index 00000000000..6d1d0d7cd27
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr93385.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-dce -fno-ipa-cp -fno-tree-dce" } */
+
+char a, b;
+
+#ifdef __SIZEOF_INT128__
+#define T unsigned __int128
+#else
+#define T unsigned
+#endif
+
+static inline int
+c (T d)
+{
+  char e = 0;
+  d %= (unsigned) d;
+  e -= 0;
+  __builtin_strncpy (&a, &e, 1);
+  return e + b;
+}
+
+int
+main (void)
+{
+  c (~0);
+  return 0;
+}
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 4f15e57da0b..f605e763f4a 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -1526,6 +1526,11 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id)
 	  : !opt_for_fn (id->dst_fn, flag_var_tracking_assignments)))
     return NULL;
 
+  if (!is_gimple_debug (stmt)
+      && id->param_body_adjs
+      && id->param_body_adjs->m_dead_stmts.contains (stmt))
+    return NULL;
+
   /* Begin by recognizing trees that we'll completely rewrite for the
      inlining context.  Our output for these trees is completely
      different from our input (e.g. RETURN_EXPR is deleted and morphs
@@ -1790,10 +1795,15 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id)
 
       if (gimple_debug_bind_p (stmt))
 	{
+	  tree value;
+	  if (id->param_body_adjs
+	      && id->param_body_adjs->m_dead_stmts.contains (stmt))
+	    value = NULL_TREE;
+	  else
+	    value = gimple_debug_bind_get_value (stmt);
 	  gdebug *copy
 	    = gimple_build_debug_bind (gimple_debug_bind_get_var (stmt),
-				       gimple_debug_bind_get_value (stmt),
-				       stmt);
+				       value, stmt);
 	  if (id->reset_location)
 	    gimple_set_location (copy, input_location);
 	  id->debug_stmts.safe_push (copy);
@@ -2675,7 +2685,9 @@ copy_phis_for_bb (basic_block bb, copy_body_data *id)
       phi = si.phi ();
       res = PHI_RESULT (phi);
       new_res = res;
-      if (!virtual_operand_p (res))
+      if (!virtual_operand_p (res)
+	  && (!id->param_body_adjs
+	      || !id->param_body_adjs->m_dead_stmts.contains (phi)))
 	{
 	  walk_tree (&new_res, copy_tree_body_r, id, NULL);
 	  if (EDGE_COUNT (new_bb->preds) == 0)


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-06-28 16:28 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-28 16:28 [gcc r12-1848] ipa-sra: Introduce a mini-DCE to tree-inline.c (PR 93385) Martin Jambor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).