[gcc r13-4687] ipa-sra: Treat REFERENCE_TYPES as always dereferencable

[gcc r13-4687] ipa-sra: Treat REFERENCE_TYPES as always dereferencable

[Martin Jambor]

https://gcc.gnu.org/g:10478270fe0c39c59eb0f35d19356a63bdf3a2ad

commit r13-4687-g10478270fe0c39c59eb0f35d19356a63bdf3a2ad
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Dec 14 00:33:06 2022 +0100

    ipa-sra: Treat REFERENCE_TYPES as always dereferencable
    
    C++ and especially Fortran pass data by references which are not
    pointers potentially pointing anywhere and so can be assumed to be
    safely dereferencable.  This patch teaches IPA-SRA to treat them as
    such and avoid the dance we do to prove that we can move loads from
    them to the caller.
    
    When we do not know that a dereference will happen all the time, we
    need a heuristics so that we do not force memory accesses that normally
    happen only rarely.  The patch simply uses the (possibly guessed)
    profile and checks whether the (expected) number of loads is at least
    half of function invocations invocations - the half is now
    configurable with a param as requested by Honza.
    
    gcc/ChangeLog:
    
    2022-12-13  Martin Jambor  <mjambor@suse.cz>
    
            PR ipa/103585
            * params.opt (ipa-sra-deref-prob-threshold): New parameter.
            * doc/invoke.texi (ipa-sra-deref-prob-threshold): Document it.
            * ipa-sra.cc (struct gensum_param_access): New field load_count.
            (struct gensum_param_desc): New field safe_ref, adjusted comments.
            (by_ref_count): Renamed to unsafe_by_ref_count, adjusted all uses.
            (dump_gensum_access): Dump the new field.
            (dump_gensum_param_descriptor): Likewise.
            (create_parameter_descriptors): Set safe_ref field, move setting
            by_ref forward.  Only increment unsafe_by_ref_count for unsafe
            by_ref parameters.
            (allocate_access): Initialize new field.
            (mark_param_dereference): Adjust indentation.  Only add data to
            bb_dereferences for unsafe by_ref parameters.
            (scan_expr_access): For loads, accumulate BB counts.
            (dereference_probable_p): New function.
            (check_gensum_access): Fix leading comment, add parameter FUN.
            Check cumulative counts of loads for safe by_ref accesses instead
            of dereferences.
            (process_scan_results): Do not propagate dereference distances for
            safe by_ref parameters.  Pass fun to check_gensum_access.  Safe
            by_ref params do not need the postdominance check.
    
    gcc/testsuite/ChangeLog:
    
    2022-11-11  Martin Jambor  <mjambor@suse.cz>
    
            * g++.dg/ipa/ipa-sra-5.C: New test

Diff:
---
 gcc/doc/invoke.texi                  |   5 ++
 gcc/ipa-sra.cc                       | 101 +++++++++++++++++++++++++----------
 gcc/params.opt                       |   4 ++
 gcc/testsuite/g++.dg/ipa/ipa-sra-5.C |  23 ++++++++
 4 files changed, 105 insertions(+), 28 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 13371972fd1..0459714d100 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -15509,6 +15509,11 @@ the parameter is reserved exclusively for debug insns created by
 @option{-fvar-tracking-assignments}, but debug insns may get
 (non-overlapping) uids above it if the reserved range is exhausted.
 
+@item ipa-sra-deref-prob-threshold
+IPA-SRA replaces a pointer which is known not be NULL with one or more
+new parameters only when the probability (in percent, relative to
+function entry) of it being dereferenced is higher than this parameter.
+
 @item ipa-sra-ptr-growth-factor
 IPA-SRA replaces a pointer to an aggregate with one or more new
 parameters only when their cumulative size is less or equal to
diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
index 0f137e810fe..866f52e89e0 100644
--- a/gcc/ipa-sra.cc
+++ b/gcc/ipa-sra.cc
@@ -151,6 +151,8 @@ struct gensum_param_access
      arguments.  */
   tree alias_ptr_type;
 
+  /* Cumulative count of all loads. */
+  profile_count load_count;
   /* Have there been writes to or reads from this exact location except for as
      arguments to a function call that can be tracked.  */
   bool nonarg;
@@ -207,8 +209,13 @@ struct gensum_param_desc
      by reference that is a candidate for being converted to a set of
      parameters passing those data by value.  */
   bool split_candidate;
-  /* Is this a parameter passing stuff by reference?  */
+  /* Is this a parameter passing stuff by reference (either a pointer or a
+     source language reference type)?  */
   bool by_ref;
+  /* If this parameter passes stuff by reference, can it be safely dereferenced
+     without performing further checks (for example because it is a
+     REFERENCE_TYPE)?  */
+  bool safe_ref;
 
   /* The number of this parameter as they are ordered in function decl.  */
   int param_number;
@@ -561,7 +568,7 @@ int aa_walking_limit;
    accessed in that BB.  */
 HOST_WIDE_INT *bb_dereferences = NULL;
 /* How many by-reference parameters there are in the current function.  */
-int by_ref_count;
+int unsafe_by_ref_count;
 
 /* Bitmap of BBs that can cause the function to "stop" progressing by
    returning, throwing externally, looping infinitely or calling a function
@@ -643,6 +650,8 @@ dump_gensum_access (FILE *f, gensum_param_access *access, unsigned indent)
   print_generic_expr (f, access->type);
   fprintf (f, ", alias_ptr_type: ");
   print_generic_expr (f, access->alias_ptr_type);
+  fprintf (f, ", load_count: ");
+  access->load_count.dump (f);
   fprintf (f, ", nonarg: %u, reverse: %u\n", access->nonarg, access->reverse);
   for (gensum_param_access *ch = access->first_child;
        ch;
@@ -692,7 +701,8 @@ dump_gensum_param_descriptor (FILE *f, gensum_param_desc *desc)
       return;
     }
   if (desc->by_ref)
-    fprintf (f, "    by_ref with %u pass throughs\n", desc->ptr_pt_count);
+    fprintf (f, "    %s by_ref with %u pass throughs\n",
+	     desc->safe_ref ? "safe" : "unsafe", desc->ptr_pt_count);
 
   for (gensum_param_access *acc = desc->accesses; acc; acc = acc->next_sibling)
     dump_gensum_access (f, acc, 2);
@@ -1140,6 +1150,8 @@ create_parameter_descriptors (cgraph_node *node,
 
       if (POINTER_TYPE_P (type))
 	{
+	  desc->by_ref = true;
+	  desc->safe_ref = (TREE_CODE (type) == REFERENCE_TYPE);
 	  type = TREE_TYPE (type);
 
 	  if (TREE_CODE (type) == FUNCTION_TYPE
@@ -1187,7 +1199,6 @@ create_parameter_descriptors (cgraph_node *node,
 			 "nonarg uses\n");
 	      continue;
 	    }
-	  desc->by_ref = true;
 	}
       else if (!AGGREGATE_TYPE_P (type))
 	{
@@ -1231,8 +1242,8 @@ create_parameter_descriptors (cgraph_node *node,
 
       ret = true;
       desc->split_candidate = true;
-      if (desc->by_ref)
-	desc->deref_index = by_ref_count++;
+      if (desc->by_ref && !desc->safe_ref)
+	desc->deref_index = unsafe_by_ref_count++;
     }
   return ret;
 }
@@ -1301,6 +1312,7 @@ allocate_access (gensum_param_desc *desc,
   memset (access, 0, sizeof (*access));
   access->offset = offset;
   access->size = size;
+  access->load_count = profile_count::zero ();
   return access;
 }
 
@@ -1555,15 +1567,16 @@ asm_visit_addr (gimple *, tree op, tree, void *)
 
 static void
 mark_param_dereference (gensum_param_desc *desc, HOST_WIDE_INT dist,
-		       basic_block bb)
+			basic_block bb)
 {
   gcc_assert (desc->by_ref);
   gcc_checking_assert (desc->split_candidate);
 
-  if (bitmap_bit_p (final_bbs, bb->index))
+  if (desc->safe_ref
+      || bitmap_bit_p (final_bbs, bb->index))
     return;
 
-  int idx = bb->index * by_ref_count + desc->deref_index;
+  int idx = bb->index * unsafe_by_ref_count + desc->deref_index;
   if (bb_dereferences[idx] < dist)
     bb_dereferences[idx] = dist;
 }
@@ -1811,6 +1824,8 @@ scan_expr_access (tree expr, gimple *stmt, isra_scan_context ctx,
 	   other.  */
 	access->nonarg = true;
     }
+  else if (ctx == ISRA_CTX_LOAD && bb->count.initialized_p ())
+    access->load_count += bb->count;
 
   if (!access->type)
     {
@@ -2092,9 +2107,9 @@ dump_dereferences_table (FILE *f, struct function *fun, const char *str)
       if (bb != EXIT_BLOCK_PTR_FOR_FN (fun))
 	{
 	  int i;
-	  for (i = 0; i < by_ref_count; i++)
+	  for (i = 0; i < unsafe_by_ref_count; i++)
 	    {
-	      int idx = bb->index * by_ref_count + i;
+	      int idx = bb->index * unsafe_by_ref_count + i;
 	      fprintf (f, " %4" HOST_WIDE_INT_PRINT "d", bb_dereferences[idx]);
 	    }
 	}
@@ -2139,15 +2154,15 @@ propagate_dereference_distances (struct function *fun)
       if (bitmap_bit_p (final_bbs, bb->index))
 	continue;
 
-      for (i = 0; i < by_ref_count; i++)
+      for (i = 0; i < unsafe_by_ref_count; i++)
 	{
-	  int idx = bb->index * by_ref_count + i;
+	  int idx = bb->index * unsafe_by_ref_count + i;
 	  bool first = true;
 	  HOST_WIDE_INT inh = 0;
 
 	  FOR_EACH_EDGE (e, ei, bb->succs)
 	  {
-	    int succ_idx = e->dest->index * by_ref_count + i;
+	    int succ_idx = e->dest->index * unsafe_by_ref_count + i;
 
 	    if (e->dest == EXIT_BLOCK_PTR_FOR_FN (fun))
 	      continue;
@@ -2184,13 +2199,24 @@ propagate_dereference_distances (struct function *fun)
 			     "Dereference table after propagation:\n");
 }
 
-/* Perform basic checks on ACCESS to PARM described by DESC and all its
-   children, return true if the parameter cannot be split, otherwise return
-   true and update *TOTAL_SIZE and *ONLY_CALLS.  ENTRY_BB_INDEX must be the
-   index of the entry BB in the function of PARM.  */
+/* Return true if the ACCESS loads happen frequently enough in FUN to risk
+   moving them to the caller and only pass the result.  */
 
 static bool
-check_gensum_access (tree parm, gensum_param_desc *desc,
+dereference_probable_p (struct function *fun, gensum_param_access *access)
+{
+  int threshold = opt_for_fn (fun->decl, param_ipa_sra_deref_prob_threshold);
+  return access->load_count
+    >= ENTRY_BLOCK_PTR_FOR_FN (fun)->count.apply_scale (threshold, 100);
+}
+
+/* Perform basic checks on ACCESS to PARM (of FUN) described by DESC and all
+   its children, return true if the parameter cannot be split, otherwise return
+   false and update *NONARG_ACC_SIZE and *ONLY_CALLS.  ENTRY_BB_INDEX must be
+   the index of the entry BB in the function of PARM.  */
+
+static bool
+check_gensum_access (struct function *fun, tree parm, gensum_param_desc *desc,
 		     gensum_param_access *access,
 		     HOST_WIDE_INT *nonarg_acc_size, bool *only_calls,
 		     int entry_bb_index)
@@ -2218,19 +2244,31 @@ check_gensum_access (tree parm, gensum_param_desc *desc,
 
   if (desc->by_ref)
     {
-      int idx = (entry_bb_index * by_ref_count + desc->deref_index);
-      if ((access->offset + access->size) > bb_dereferences[idx])
+      if (desc->safe_ref)
 	{
-	  disqualify_split_candidate (desc, "Would create a possibly "
-				      "illegal dereference in a caller.");
-	  return true;
+	  if (!dereference_probable_p (fun, access))
+	    {
+	      disqualify_split_candidate (desc, "Dereferences in callers "
+					  "would happen much more frequently.");
+	      return true;
+	    }
+	}
+      else
+	{
+	  int idx = (entry_bb_index * unsafe_by_ref_count + desc->deref_index);
+	  if ((access->offset + access->size) > bb_dereferences[idx])
+	    {
+	      disqualify_split_candidate (desc, "Would create a possibly "
+					  "illegal dereference in a caller.");
+	      return true;
+	    }
 	}
     }
 
   for (gensum_param_access *ch = access->first_child;
        ch;
        ch = ch->next_sibling)
-    if (check_gensum_access (parm, desc, ch, nonarg_acc_size, only_calls,
+    if (check_gensum_access (fun, parm, desc, ch, nonarg_acc_size, only_calls,
 			     entry_bb_index))
       return true;
 
@@ -2288,6 +2326,7 @@ process_scan_results (cgraph_node *node, struct function *fun,
 
       if (!dereferences_propagated
 	  && desc->by_ref
+	  && !desc->safe_ref
 	  && desc->accesses)
 	{
 	  propagate_dereference_distances (fun);
@@ -2302,8 +2341,8 @@ process_scan_results (cgraph_node *node, struct function *fun,
       for (gensum_param_access *acc = desc->accesses;
 	   acc;
 	   acc = acc->next_sibling)
-	if (check_gensum_access (parm, desc, acc, &nonarg_acc_size, &only_calls,
-				 entry_bb_index))
+	if (check_gensum_access (fun, parm, desc, acc, &nonarg_acc_size,
+				 &only_calls, entry_bb_index))
 	  {
 	    check_failed = true;
 	    break;
@@ -2394,6 +2433,12 @@ process_scan_results (cgraph_node *node, struct function *fun,
 	    if (!uses_memory_as_obtained)
 	      continue;
 
+	    if (desc->safe_ref)
+	      {
+		csum->m_arg_flow[argidx].safe_to_import_accesses = true;
+		continue;
+	      }
+
 	    /* Post-dominator check placed last, hoping that it usually won't
 	       be needed.  */
 	    if (!pdoms_calculated)
@@ -4124,7 +4169,7 @@ ipa_sra_summarize_function (cgraph_node *node)
 	  cfun_pushed = true;
 	  final_bbs = BITMAP_ALLOC (NULL);
 	  bb_dereferences = XCNEWVEC (HOST_WIDE_INT,
-				      by_ref_count
+				      unsafe_by_ref_count
 				      * last_basic_block_for_fn (fun));
 	  aa_walking_limit = opt_for_fn (node->decl, param_ipa_max_aa_steps);
 	  scan_function (node, fun);
diff --git a/gcc/params.opt b/gcc/params.opt
index 397ec0bd128..a574f17bddb 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -282,6 +282,10 @@ Maximum number of different predicates used to track properties of loops in IPA
 Common Joined UInteger Var(param_ipa_max_switch_predicate_bounds) Init(5) Param Optimization
 Maximal number of boundary endpoints of case ranges of switch statement used during IPA function summary generation.
 
+-param=ipa-sra-deref-prob-threshold=
+Common Joined UInteger Var(param_ipa_sra_deref_prob_threshold) Init(50) IntegerRange(0, 100) Param Optimization
+Minimum probability (in percent) of dereferencing of a function pointer parameter for it to be considered for replacement with simple values.
+
 -param=ipa-sra-max-replacements=
 Common Joined UInteger Var(param_ipa_sra_max_replacements) Optimization Init(8) IntegerRange(0, 16) Param
 Maximum pieces that IPA-SRA tracks per formal parameter, as a consequence, also the maximum number of replacements of a formal parameter.
diff --git a/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C b/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C
new file mode 100644
index 00000000000..26a221e1734
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/ipa-sra-5.C
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-ipa-sra"  } */
+
+volatile int vi;
+
+static void __attribute__((noinline))
+foo (int c, int &r)
+{
+  int i;
+  if (c)
+    i = r;
+  else
+    i = 0;
+  vi = i;
+}
+
+void
+bar (int c, int j)
+{
+  foo (c, j);
+}
+
+/* { dg-final { scan-ipa-dump "Will split parameter" "sra" } } */