public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-7160] AutoFDO: Don't try to promote indirect calls that result in recursive direct calls
@ 2022-02-10  7:34 Eugene Rozenfeld
  0 siblings, 0 replies; only message in thread
From: Eugene Rozenfeld @ 2022-02-10  7:34 UTC (permalink / raw)
  To: gcc-cvs

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

commit r12-7160-gba125745d9e9fe90a18a2af8701b3269c5fdd468
Author: Eugene Rozenfeld <erozen@microsoft.com>
Date:   Tue Feb 8 23:00:33 2022 -0800

    AutoFDO: Don't try to promote indirect calls that result in recursive direct calls
    
    AutoFDO tries to promote and inline all indirect calls that were promoted
    and inlined in the original binary and that are still hot. In the included
    test case, the promotion results in a direct call that is a recursive call.
    inline_call and optimize_inline_calls can't handle recursive calls at this stage.
    Currently, inline_call fails with a segmentation fault.
    
    This change leaves the indirect call alone if promotion will result in a recursive call.
    
    Tested on x86_64-pc-linux-gnu.
    
    gcc/ChangeLog:
            * auto-profile.cc (afdo_indirect_call): Don't attempt to promote indirect calls
            that will result in direct recursive calls.
    
    gcc/testsuite/ChangeLog:
            * g++.dg/tree-prof/indir-call-recursive-inlining.C : New test.

Diff:
---
 gcc/auto-profile.cc                                | 40 +++++++++-------
 .../tree-prof/indir-call-recursive-inlining.C      | 54 ++++++++++++++++++++++
 2 files changed, 78 insertions(+), 16 deletions(-)

diff --git a/gcc/auto-profile.cc b/gcc/auto-profile.cc
index c7cee639c85..2b34b80b82d 100644
--- a/gcc/auto-profile.cc
+++ b/gcc/auto-profile.cc
@@ -975,7 +975,7 @@ read_profile (void)
      * after annotation, we just need to mark, and let follow-up logic to
        decide if it needs to promote and inline.  */
 
-static void
+static bool
 afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
                     bool transform)
 {
@@ -983,12 +983,12 @@ afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
   tree callee;
 
   if (map.size () == 0)
-    return;
+    return false;
   gcall *stmt = dyn_cast <gcall *> (gs);
   if (!stmt
       || gimple_call_internal_p (stmt)
       || gimple_call_fndecl (stmt) != NULL_TREE)
-    return;
+    return false;
 
   gcov_type total = 0;
   icall_target_map::const_iterator max_iter = map.end ();
@@ -1003,7 +1003,7 @@ afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
   struct cgraph_node *direct_call = cgraph_node::get_for_asmname (
       get_identifier (afdo_string_table->get_name (max_iter->first)));
   if (direct_call == NULL || !direct_call->profile_id)
-    return;
+    return false;
 
   callee = gimple_call_fn (stmt);
 
@@ -1013,20 +1013,27 @@ afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
   hist->hvalue.counters = XNEWVEC (gcov_type, hist->n_counters);
   gimple_add_histogram_value (cfun, stmt, hist);
 
-  // Total counter
+  /* Total counter */
   hist->hvalue.counters[0] = total;
-  // Number of value/counter pairs
+  /* Number of value/counter pairs */
   hist->hvalue.counters[1] = 1;
-  // Value
+  /* Value */
   hist->hvalue.counters[2] = direct_call->profile_id;
-  // Counter
+  /* Counter */
   hist->hvalue.counters[3] = max_iter->second;
 
   if (!transform)
-    return;
+    return false;
+
+  cgraph_node* current_function_node = cgraph_node::get (current_function_decl);
+
+  /* If the direct call is a recursive call, don't promote it since
+     we are not set up to inline recursive calls at this stage. */
+  if (direct_call == current_function_node)
+    return false;
 
   struct cgraph_edge *indirect_edge
-      = cgraph_node::get (current_function_decl)->get_edge (stmt);
+      = current_function_node->get_edge (stmt);
 
   if (dump_file)
     {
@@ -1040,13 +1047,13 @@ afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
     {
       if (dump_file)
         fprintf (dump_file, " not transforming\n");
-      return;
+      return false;
     }
   if (DECL_STRUCT_FUNCTION (direct_call->decl) == NULL)
     {
       if (dump_file)
         fprintf (dump_file, " no declaration\n");
-      return;
+      return false;
     }
 
   if (dump_file)
@@ -1063,16 +1070,17 @@ afdo_indirect_call (gimple_stmt_iterator *gsi, const icall_target_map &map,
   cgraph_edge::redirect_call_stmt_to_callee (new_edge);
   gimple_remove_histogram_value (cfun, stmt, hist);
   inline_call (new_edge, true, NULL, NULL, false);
+  return true;
 }
 
 /* From AutoFDO profiles, find values inside STMT for that we want to measure
    histograms and adds them to list VALUES.  */
 
-static void
+static bool
 afdo_vpt (gimple_stmt_iterator *gsi, const icall_target_map &map,
           bool transform)
 {
-  afdo_indirect_call (gsi, map, transform);
+  return afdo_indirect_call (gsi, map, transform);
 }
 
 typedef std::set<basic_block> bb_set;
@@ -1498,8 +1506,8 @@ afdo_vpt_for_early_inline (stmt_set *promoted_stmts)
           {
             /* Promote the indirect call and update the promoted_stmts.  */
             promoted_stmts->insert (stmt);
-            afdo_vpt (&gsi, info.targets, true);
-            has_vpt = true;
+            if (afdo_vpt (&gsi, info.targets, true))
+              has_vpt = true;
           }
       }
   }
diff --git a/gcc/testsuite/g++.dg/tree-prof/indir-call-recursive-inlining.C b/gcc/testsuite/g++.dg/tree-prof/indir-call-recursive-inlining.C
new file mode 100644
index 00000000000..11f690063ef
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-prof/indir-call-recursive-inlining.C
@@ -0,0 +1,54 @@
+/* { dg-options "-O2 " } */
+
+class Parent
+{
+public:
+  Parent *object;
+
+  Parent()
+  {
+       object = this;
+  }
+
+  virtual void recurse (int t) = 0;
+};
+
+class Child : public Parent
+{
+
+  Parent *
+  get_object ()
+  {
+     return this;
+  }
+
+public:
+  virtual void
+  recurse (int t)
+  {
+    if (t != 10)
+      for (int i = 0; i < 5; ++i)
+        get_object()->recurse(t + 1);
+  };
+};
+
+Parent *
+create_object ()
+{
+  Child *mod = new Child;
+  return mod;
+}
+
+int
+main (int argc, char **argv)
+{
+  Parent *parent = create_object ();
+
+  for (int i = 0; i < 5; ++i)
+    {
+	  parent->recurse (0);
+    }
+
+  return 0;
+}
+


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

only message in thread, other threads:[~2022-02-10  7:34 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10  7:34 [gcc r12-7160] AutoFDO: Don't try to promote indirect calls that result in recursive direct calls Eugene Rozenfeld

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).