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 --git a/gcc/cgraph.h b/gcc/cgraph.h
index 8c512b648ee586bf319da23d271d7749673479d2..d96690326d14c45e4dde42e43fb5e8eaa0f33d50 100644 (file)
--- 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 e3afd8a40472c0d273d81affb13875b74177721a..fbb31f6dff2ab5dcad7d4d3fb7a0283b19e74d72 100644 (file)
--- 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 55c9c37570264d7495842aeeece1a0dd59a92311..0197ac6108df68bd1fd0ac4a70bae2e6969cbddb 100644 (file)
--- 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 b22dfb5315ccdb89cc5c112fca78284a81548697..0c50ead07d9e6015d40611251073241056e6cf7c 100644 (file)
--- 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 8670337416e8a1459834578b3790b2d3602bd487..c6722a708e24927d9827445e8b5bd89b24c19e71 100644 (file)
--- 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 (file)
index 0000000..c5d9e3d
--- /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 (file)
index 0000000..7a49ea6
--- /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;
+}