dom_oracle::register_transitives contains an unbound dominator walk
which for the testcase in PR114855 dominates the profile.  I've also
noticed odd behavior in the case when set_one_relation returns NULL,
we'd then completely abort processing other relations.  The following
fixes the latter by continuing searching and fixes the unbound work
done by assigning a constant work budget to the loop, bounding the
number of dominators visited but also the number of relations
processed.  This gets both dom_oracle::register_transitives and
get_immediate_dominator off the profile.

I'll note that we're still doing an unbound dominator walk via
equiv_set in find_equiv_dom at the start of the function and when
we register a relation that also looks up the same way.  At least
for the testcase at hand this isn't an issue.

I've wondered whether instead of having a per-BB bitmap of names
we have a relation for in that BB having a bitmap per SSA name of
which BBs have a relation for it would be a way to avoid all those
walks.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK for trunk?

Thanks,
Richard.

        PR tree-optimization/114855
        * params.opt (--param transitive-relations-work-bound): New.
        * doc/invoke.texi (--param transitive-relations-work-bound):
        Document.
        * value-relation.cc (dom_oracle::register_transitives):
        Do not stop processing relations when a registration attempt
        did not register a new relation.  Assing an overall work
        budget, bounding the dominator walk and the number of
        relations processed.
---
 gcc/doc/invoke.texi   |  3 +++
 gcc/params.opt        |  4 ++++
 gcc/value-relation.cc | 47 ++++++++++++++++++++++++++-----------------
 3 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index bdbbea53666..bd1208a62ee 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -17144,6 +17144,9 @@ in the outgoing range calculator.
 @item relation-block-limit
 Maximum number of relations the oracle will register in a basic block.
 
+@item transitive-relations-work-bound
+Work bound when discovering transitive relations from existing relations.
+
 @item min-pagesize
 Minimum page size for warning purposes.
 
diff --git a/gcc/params.opt b/gcc/params.opt
index 949b4754498..a08e4c1042d 100644
--- a/gcc/params.opt
+++ b/gcc/params.opt
@@ -924,6 +924,10 @@ outgoing range calculator.
 Common Joined UInteger Var(param_relation_block_limit) Init(200) 
IntegerRange(0, 9999) Param Optimization
 Maximum number of relations the oracle will register in a basic block.
 
+-param=transitive-relations-work-bound=
+Common Joined UInteger Var(param_transitive_relations_work_bound) Init(256) 
IntegerRange(0, 9999) Param Optimization
+Work bound when discovering transitive relations from existing relations.
+
 -param=rpo-vn-max-loop-depth=
 Common Joined UInteger Var(param_rpo_vn_max_loop_depth) Init(7) 
IntegerRange(2, 65536) Param Optimization
 Maximum depth of a loop nest to fully value-number optimistically.
diff --git a/gcc/value-relation.cc b/gcc/value-relation.cc
index d6ad2dd984f..b01e2953188 100644
--- a/gcc/value-relation.cc
+++ b/gcc/value-relation.cc
@@ -1204,7 +1204,13 @@ dom_oracle::register_transitives (basic_block root_bb,
   const_bitmap equiv1 = equiv_set (relation.op1 (), root_bb);
   const_bitmap equiv2 = equiv_set (relation.op2 (), root_bb);
 
-  for (bb = root_bb; bb; bb = get_immediate_dominator (CDI_DOMINATORS, bb))
+  const unsigned work_budget = param_transitive_relations_work_bound;
+  unsigned avail_budget = work_budget;
+  for (bb = root_bb; bb;
+       /* Advancing to the next immediate dominator eats from the budget,
+         if none is left after that there's no point to continue.  */
+       bb = (--avail_budget > 0
+            ? get_immediate_dominator (CDI_DOMINATORS, bb) : nullptr))
     {
       int bbi = bb->index;
       if (bbi >= (int)m_relations.length())
@@ -1247,27 +1253,30 @@ dom_oracle::register_transitives (basic_block root_bb,
 
          // Ignore if both NULL (not relevant relation) or the same,
          if (r1 == r2)
-           continue;
+           ;
 
-         // Any operand not an equivalence, just take the real operand.
-         if (!r1)
-           r1 = relation.op1 ();
-         if (!r2)
-           r2 = relation.op2 ();
-
-         value_relation nr (relation.kind (), r1, r2);
-         if (nr.apply_transitive (*ptr))
+         else
            {
-             if (!set_one_relation (root_bb, nr.kind (), nr.op1 (), nr.op2 ()))
-               return;
-             if (dump_file && (dump_flags & TDF_DETAILS))
-               {
-                 fprintf (dump_file, "   Registering transitive relation ");
-                 nr.dump (dump_file);
-                 fputc ('\n', dump_file);
-               }
+             // Any operand not an equivalence, just take the real operand.
+             if (!r1)
+               r1 = relation.op1 ();
+             if (!r2)
+               r2 = relation.op2 ();
+
+             value_relation nr (relation.kind (), r1, r2);
+             if (nr.apply_transitive (*ptr)
+                 && set_one_relation (root_bb, nr.kind (),
+                                      nr.op1 (), nr.op2 ()))
+               if (dump_file && (dump_flags & TDF_DETAILS))
+                 {
+                   fprintf (dump_file, "   Registering transitive relation ");
+                   nr.dump (dump_file);
+                   fputc ('\n', dump_file);
+                 }
            }
-
+         /* Processed one relation, abort if we've eaten up our budget.  */
+         if (--avail_budget == 0)
+           return;
        }
     }
 }
-- 
2.43.0

Reply via email to