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.

diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index bfea971..f5f5476 100644 (file)
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -196,10 +196,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 6477bb8..b3e0f62 100644 (file)
--- a/gcc/ipa-cp.cc
+++ b/gcc/ipa-cp.cc
@@ -4348,7 +4348,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 "
@@ -5180,10 +5181,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 ());
@@ -5191,7 +5194,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);
@@ -5218,6 +5222,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;
 
@@ -5225,8 +5233,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 83a1816..0d81674 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.  */
@@ -3314,7 +3319,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:
@@ -3671,7 +3682,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;
 
@@ -4130,7 +4141,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);
@@ -4158,7 +4170,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 "
@@ -4206,7 +4219,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)
@@ -4432,7 +4445,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);
 
@@ -4692,6 +4706,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 a40ccc0..7eb5c8f 100644 (file)
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -115,6 +115,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
@@ -362,6 +365,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.  */
 
 inline tree
@@ -399,6 +411,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 cfc8b79..0470509 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;
+}