public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-9476] ipa: Fix double reference-count decrements for the same edge (PR 107769, PR 109318)
@ 2023-04-26 16:45 Martin Jambor
  0 siblings, 0 replies; only message in thread
From: Martin Jambor @ 2023-04-26 16:45 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:bea3885200c549419567ad3a43ac71642619ad1a

commit r12-9476-gbea3885200c549419567ad3a43ac71642619ad1a
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Apr 26 18:38:39 2023 +0200

    ipa: Fix double reference-count decrements for the same edge (PR 107769, PR 109318)
    
    It turns out that since addition of the code that can identify globals
    which are only read from, the code that keeps track of the references
    can decrement their count for the same calls, once during IPA-CP and
    then again during inlining.  Fixed by adding a special flag to the
    pass-through variant and simply wiping out the reference to the
    refdesc structure from the constant ones.
    
    Moreover, during debugging of the issue I have discovered that the
    code removing references could remove a reference associated with the
    same statement but of a wrong type.  In all cases it wanted to remove
    an IPA_REF_ADDR reference so removing a lesser one instead should do
    no harm in practice, but we should try to be consistent and so this
    patch extends symtab_node::find_reference so that it searches for a
    reference of a given type only.
    
    gcc/ChangeLog:
    
    2023-04-14  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/107769
            PR ipa/109318
            * cgraph.h (symtab_node::find_reference): Add parameter use_type.
            * ipa-prop.h (ipa_pass_through_data): New flag refdesc_decremented.
            (ipa_zap_jf_refdesc): New function.
            (ipa_get_jf_pass_through_refdesc_decremented): Likewise.
            (ipa_set_jf_pass_through_refdesc_decremented): Likewise.
            * ipa-cp.cc (ipcp_discover_new_direct_edges): Provide a value for
            the new parameter of find_reference.
            (adjust_references_in_caller): Likewise. Make sure the constant jump
            function is not used to decrement a refdec counter again.  Only
            decrement refdesc counters when the pass_through jump function allows
            it.  Added a detailed dump when decrementing refdesc counters.
            * ipa-prop.cc (ipa_print_node_jump_functions_for_edge): Dump new flag.
            (ipa_set_jf_simple_pass_through): Initialize the new flag.
            (ipa_set_jf_unary_pass_through): Likewise.
            (ipa_set_jf_arith_pass_through): Likewise.
            (remove_described_reference): Provide a value for the new parameter of
            find_reference.
            (update_jump_functions_after_inlining): Zap refdesc of new jfunc if
            the previous pass_through had a flag mandating that we do so.
            (propagate_controlled_uses): Likewise.  Only decrement refdesc
            counters when the pass_through jump function allows it.
            (ipa_edge_args_sum_t::duplicate): Provide a value for the new
            parameter of find_reference.
            (ipa_write_jump_function): Assert the new flag does not have to be
            streamed.
            * symtab.cc (symtab_node::find_reference): Add parameter use_type, use
            it in searching.
    
    gcc/testsuite/ChangeLog:
    
    2023-04-06  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/107769
            PR ipa/109318
            * gcc.dg/ipa/pr109318.c: New test.
            * gcc.dg/lto/pr107769_0.c: Likewise.
    
    (cherry picked from commit 8e08c7886eed5824bebd0e011526ec302d622844)

Diff:
---
 gcc/cgraph.h                          |  7 ++---
 gcc/ipa-cp.cc                         | 18 +++++++++----
 gcc/ipa-prop.cc                       | 27 +++++++++++++++-----
 gcc/ipa-prop.h                        | 32 +++++++++++++++++++++++
 gcc/symtab.cc                         |  8 +++---
 gcc/testsuite/gcc.dg/ipa/pr109318.c   | 20 +++++++++++++++
 gcc/testsuite/gcc.dg/lto/pr107769_0.c | 48 +++++++++++++++++++++++++++++++++++
 7 files changed, 143 insertions(+), 17 deletions(-)

diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index 8c512b648ee..d96690326d1 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -193,10 +193,11 @@ public:
   /* Clone reference REF to this symtab_node and set its stmt to STMT.  */
   ipa_ref *clone_reference (ipa_ref *ref, gimple *stmt);
 
-  /* Find the structure describing a reference to REFERRED_NODE
-     and associated with statement STMT.  */
+  /* Find the structure describing a reference to REFERRED_NODE of USE_TYPE and
+     associated with statement STMT or LTO_STMT_UID.  */
   ipa_ref *find_reference (symtab_node *referred_node, gimple *stmt,
-			   unsigned int lto_stmt_uid);
+			   unsigned int lto_stmt_uid,
+			   enum ipa_ref_use use_type);
 
   /* Remove all references that are associated with statement STMT.  */
   void remove_stmt_references (gimple *stmt);
diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc
index e3afd8a4047..fbb31f6dff2 100644
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -4139,7 +4139,8 @@ ipcp_discover_new_direct_edges (struct cgraph_node *node,
 		    fprintf (dump_file, "     controlled uses count of param "
 			     "%i bumped down to %i\n", param_index, c);
 		  if (c == 0
-		      && (to_del = node->find_reference (cs->callee, NULL, 0)))
+		      && (to_del = node->find_reference (cs->callee, NULL, 0,
+							 IPA_REF_ADDR)))
 		    {
 		      if (dump_file && (dump_flags & TDF_DETAILS))
 			fprintf (dump_file, "       and even removing its "
@@ -4954,10 +4955,12 @@ adjust_references_in_caller (cgraph_edge *cs, symtab_node *symbol, int index)
   if (jfunc->type == IPA_JF_CONST)
     {
       ipa_ref *to_del = cs->caller->find_reference (symbol, cs->call_stmt,
-						    cs->lto_stmt_uid);
+						    cs->lto_stmt_uid,
+						    IPA_REF_ADDR);
       if (!to_del)
 	return;
       to_del->remove_reference ();
+      ipa_zap_jf_refdesc (jfunc);
       if (dump_file)
 	fprintf (dump_file, "    Removed a reference from %s to %s.\n",
 		 cs->caller->dump_name (), symbol->dump_name ());
@@ -4965,7 +4968,8 @@ adjust_references_in_caller (cgraph_edge *cs, symtab_node *symbol, int index)
     }
 
   if (jfunc->type != IPA_JF_PASS_THROUGH
-      || ipa_get_jf_pass_through_operation (jfunc) != NOP_EXPR)
+      || ipa_get_jf_pass_through_operation (jfunc) != NOP_EXPR
+      || ipa_get_jf_pass_through_refdesc_decremented (jfunc))
     return;
 
   int fidx = ipa_get_jf_pass_through_formal_id (jfunc);
@@ -4992,6 +4996,10 @@ adjust_references_in_caller (cgraph_edge *cs, symtab_node *symbol, int index)
   gcc_assert (cuses > 0);
   cuses--;
   ipa_set_controlled_uses (caller_info, fidx, cuses);
+  ipa_set_jf_pass_through_refdesc_decremented (jfunc, true);
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    fprintf (dump_file, "    Controlled uses of parameter %i of %s dropped "
+	     "to %i.\n", fidx, caller->dump_name (), cuses);
   if (cuses)
     return;
 
@@ -4999,8 +5007,8 @@ adjust_references_in_caller (cgraph_edge *cs, symtab_node *symbol, int index)
     {
       /* Cloning machinery has created a reference here, we need to either
 	 remove it or change it to a read one.  */
-      ipa_ref *to_del = caller->find_reference (symbol, NULL, 0);
-      if (to_del && to_del->use == IPA_REF_ADDR)
+      ipa_ref *to_del = caller->find_reference (symbol, NULL, 0, IPA_REF_ADDR);
+      if (to_del)
 	{
 	  to_del->remove_reference ();
 	  if (dump_file)
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index 55c9c375702..0197ac6108d 100644
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -347,6 +347,8 @@ ipa_print_node_jump_functions_for_edge (FILE *f, struct cgraph_edge *cs)
 	    }
 	  if (jump_func->value.pass_through.agg_preserved)
 	    fprintf (f, ", agg_preserved");
+	  if (jump_func->value.pass_through.refdesc_decremented)
+	    fprintf (f, ", refdesc_decremented");
 	  fprintf (f, "\n");
 	}
       else if (type == IPA_JF_ANCESTOR)
@@ -572,6 +574,7 @@ ipa_set_jf_simple_pass_through (struct ipa_jump_func *jfunc, int formal_id,
   jfunc->value.pass_through.formal_id = formal_id;
   jfunc->value.pass_through.operation = NOP_EXPR;
   jfunc->value.pass_through.agg_preserved = agg_preserved;
+  jfunc->value.pass_through.refdesc_decremented = false;
 }
 
 /* Set JFUNC to be an unary pass through jump function.  */
@@ -585,6 +588,7 @@ ipa_set_jf_unary_pass_through (struct ipa_jump_func *jfunc, int formal_id,
   jfunc->value.pass_through.formal_id = formal_id;
   jfunc->value.pass_through.operation = operation;
   jfunc->value.pass_through.agg_preserved = false;
+  jfunc->value.pass_through.refdesc_decremented = false;
 }
 /* Set JFUNC to be an arithmetic pass through jump function.  */
 
@@ -597,6 +601,7 @@ ipa_set_jf_arith_pass_through (struct ipa_jump_func *jfunc, int formal_id,
   jfunc->value.pass_through.formal_id = formal_id;
   jfunc->value.pass_through.operation = operation;
   jfunc->value.pass_through.agg_preserved = false;
+  jfunc->value.pass_through.refdesc_decremented = false;
 }
 
 /* Set JFUNC to be an ancestor jump function.  */
@@ -3303,7 +3308,13 @@ update_jump_functions_after_inlining (struct cgraph_edge *cs,
 		  ipa_set_jf_unknown (dst);
 		  break;
 		case IPA_JF_CONST:
-		  ipa_set_jf_cst_copy (dst, src);
+		  {
+		    bool rd = ipa_get_jf_pass_through_refdesc_decremented (dst);
+		    ipa_set_jf_cst_copy (dst, src);
+		    if (rd)
+		      ipa_zap_jf_refdesc (dst);
+		  }
+
 		  break;
 
 		case IPA_JF_PASS_THROUGH:
@@ -3683,7 +3694,7 @@ remove_described_reference (symtab_node *symbol, struct ipa_cst_ref_desc *rdesc)
   if (!origin)
     return false;
   to_del = origin->caller->find_reference (symbol, origin->call_stmt,
-					   origin->lto_stmt_uid);
+					   origin->lto_stmt_uid, IPA_REF_ADDR);
   if (!to_del)
     return false;
 
@@ -4147,7 +4158,8 @@ propagate_controlled_uses (struct cgraph_edge *cs)
       struct ipa_jump_func *jf = ipa_get_ith_jump_func (args, i);
       struct ipa_cst_ref_desc *rdesc;
 
-      if (jf->type == IPA_JF_PASS_THROUGH)
+      if (jf->type == IPA_JF_PASS_THROUGH
+	  && !ipa_get_jf_pass_through_refdesc_decremented (jf))
 	{
 	  int src_idx, c, d;
 	  src_idx = ipa_get_jf_pass_through_formal_id (jf);
@@ -4175,7 +4187,8 @@ propagate_controlled_uses (struct cgraph_edge *cs)
 	      if (t && TREE_CODE (t) == ADDR_EXPR
 		  && TREE_CODE (TREE_OPERAND (t, 0)) == FUNCTION_DECL
 		  && (n = cgraph_node::get (TREE_OPERAND (t, 0)))
-		  && (ref = new_root->find_reference (n, NULL, 0)))
+		  && (ref = new_root->find_reference (n, NULL, 0,
+						      IPA_REF_ADDR)))
 		{
 		  if (dump_file)
 		    fprintf (dump_file, "ipa-prop: Removing cloning-created "
@@ -4223,7 +4236,7 @@ propagate_controlled_uses (struct cgraph_edge *cs)
 			 && clone != rdesc->cs->caller)
 		    {
 		      struct ipa_ref *ref;
-		      ref = clone->find_reference (n, NULL, 0);
+		      ref = clone->find_reference (n, NULL, 0, IPA_REF_ADDR);
 		      if (ref)
 			{
 			  if (dump_file)
@@ -4449,7 +4462,8 @@ ipa_edge_args_sum_t::duplicate (cgraph_edge *src, cgraph_edge *dst,
 		   gcc_checking_assert (n);
 		   ipa_ref *ref
 		     = src->caller->find_reference (n, src->call_stmt,
-						    src->lto_stmt_uid);
+						    src->lto_stmt_uid,
+						    IPA_REF_ADDR);
 		   gcc_checking_assert (ref);
 		   dst->caller->clone_reference (ref, ref->stmt);
 
@@ -4753,6 +4767,7 @@ ipa_write_jump_function (struct output_block *ob,
 	  streamer_write_uhwi (ob, jump_func->value.pass_through.formal_id);
 	  bp = bitpack_create (ob->main_stream);
 	  bp_pack_value (&bp, jump_func->value.pass_through.agg_preserved, 1);
+	  gcc_assert (!jump_func->value.pass_through.refdesc_decremented);
 	  streamer_write_bitpack (&bp);
 	}
       else if (TREE_CODE_CLASS (jump_func->value.pass_through.operation)
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index b22dfb5315c..0c50ead07d9 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -110,6 +110,9 @@ struct GTY(()) ipa_pass_through_data
      ipa_agg_jump_function).  The flag is used only when the operation is
      NOP_EXPR.  */
   unsigned agg_preserved : 1;
+  /* Set when the edge has already been used to decrement an appropriate
+     reference description counter and should not be decremented again.  */
+  unsigned refdesc_decremented : 1;
 };
 
 /* Structure holding data required to describe a load-value-from-aggregate
@@ -356,6 +359,15 @@ ipa_get_jf_constant_rdesc (struct ipa_jump_func *jfunc)
   return jfunc->value.constant.rdesc;
 }
 
+/* Make JFUNC not participate in any further reference counting.  */
+
+inline void
+ipa_zap_jf_refdesc (ipa_jump_func *jfunc)
+{
+  gcc_checking_assert (jfunc->type == IPA_JF_CONST);
+  jfunc->value.constant.rdesc = NULL;
+}
+
 /* Return the operand of a pass through jmp function JFUNC.  */
 
 static inline tree
@@ -393,6 +405,26 @@ ipa_get_jf_pass_through_agg_preserved (struct ipa_jump_func *jfunc)
   return jfunc->value.pass_through.agg_preserved;
 }
 
+/* Return the refdesc_decremented flag of a pass through jump function
+   JFUNC.  */
+
+inline bool
+ipa_get_jf_pass_through_refdesc_decremented (struct ipa_jump_func *jfunc)
+{
+  gcc_checking_assert (jfunc->type == IPA_JF_PASS_THROUGH);
+  return jfunc->value.pass_through.refdesc_decremented;
+}
+
+/* Set the refdesc_decremented flag of a pass through jump function JFUNC to
+   VALUE.  */
+
+inline void
+ipa_set_jf_pass_through_refdesc_decremented (ipa_jump_func *jfunc, bool value)
+{
+  gcc_checking_assert (jfunc->type == IPA_JF_PASS_THROUGH);
+  jfunc->value.pass_through.refdesc_decremented = value;
+}
+
 /* Return true if pass through jump function JFUNC preserves type
    information.  */
 
diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index 8670337416e..c6722a708e2 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -748,12 +748,13 @@ symtab_node::clone_reference (ipa_ref *ref, gimple *stmt)
   return ref2;
 }
 
-/* Find the structure describing a reference to REFERRED_NODE
-   and associated with statement STMT.  */
+/* Find the structure describing a reference to REFERRED_NODE of USE_TYPE and
+   associated with statement STMT or LTO_STMT_UID.  */
 
 ipa_ref *
 symtab_node::find_reference (symtab_node *referred_node,
-			     gimple *stmt, unsigned int lto_stmt_uid)
+			     gimple *stmt, unsigned int lto_stmt_uid,
+			     enum ipa_ref_use use_type)
 {
   ipa_ref *r = NULL;
   int i;
@@ -761,6 +762,7 @@ symtab_node::find_reference (symtab_node *referred_node,
   for (i = 0; iterate_reference (i, r); i++)
     if (r->referred == referred_node
 	&& !r->speculative
+	&& r->use == use_type
 	&& ((stmt && r->stmt == stmt)
 	    || (lto_stmt_uid && r->lto_stmt_uid == lto_stmt_uid)
 	    || (!stmt && !lto_stmt_uid && !r->stmt && !r->lto_stmt_uid)))
diff --git a/gcc/testsuite/gcc.dg/ipa/pr109318.c b/gcc/testsuite/gcc.dg/ipa/pr109318.c
new file mode 100644
index 00000000000..c5d9e3d12c7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr109318.c
@@ -0,0 +1,20 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-early-inlining" } */
+
+#pragma pack(1)
+struct S {
+  signed : 31;
+  unsigned f4 : 20;
+};
+
+static struct S global;
+
+static struct S func_16(struct S *ptr) { return *ptr; }
+
+int
+main()
+{
+  struct S *local = &global;
+  *local = func_16(local);
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr107769_0.c b/gcc/testsuite/gcc.dg/lto/pr107769_0.c
new file mode 100644
index 00000000000..7a49ea62523
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr107769_0.c
@@ -0,0 +1,48 @@
+/* { dg-lto-do run } */
+/* { dg-lto-options { { -flto -O2 -finline-limit=150 } } } */
+
+[[gnu::noipa]]
+void hjj (unsigned int lk)
+{
+    (void)lk;
+}
+void nn(int i, int n);
+[[gnu::noinline]]
+int ll(void) {
+    return 1;
+}
+void hh(int* dest, int src)
+{
+    if (!ll() && !src)
+        hjj(100);
+    (*dest) = 1;
+}
+void gg(int* result, int x)
+{
+    if (x >= 0)
+        return;
+
+    int xx;
+    xx = *result;
+    hh(result, ll());
+    if (xx >= *result)
+        nn(xx, *result);
+}
+void nn(int i, int n) {
+    int T8_;
+    if (n < 0)
+        __builtin_exit(0);
+    T8_ = 0;
+    gg(&T8_, i);
+    __builtin_exit(0);
+}
+void kk(int* x, int i) {
+    hh(x, ll());
+    if (i < 0 || i >= *x)
+        nn(i,*x);
+}
+int g__r_1 = 0;
+int main() {
+    kk(&g__r_1, 0);
+    return 0;
+}

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

only message in thread, other threads:[~2023-04-26 16:45 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-26 16:45 [gcc r12-9476] ipa: Fix double reference-count decrements for the same edge (PR 107769, PR 109318) 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).