On Thu, 13 Sep 2012, Steven Bosscher wrote:

> On Wed, Sep 12, 2012 at 4:52 PM, Steven Bosscher wrote:
> > On Wed, Sep 12, 2012 at 4:02 PM, Richard Guenther wrote:
> >> for a followup (and I bet sth else than PRE blows up at -O2 as well).
> >
> > Actually, the only thing that really blows up is that enemy of scalability, 
> > VRP.
> 
> FWIW, this appears to be due to the well-known problem with the equiv
> set, but also due to the liveness computations that tree-vrp performs,
> since your commit in r139263.
> 
> Any reason why you didn't just re-use the tree-ssa-live machinery?

Probably I didn't know about it or didn't want to keep the full life
problem life (it tries to free things as soon as possible).

> And any reason why you don't let a DEF kill a live SSA name? AFAICT
> you're exposing all SSA names up without ever killing a USE :-)

Eh ;)  We also should traverse blocks backward I suppose.  Also
the RPO traversal suffers from the same issue I noticed in PRE
and for what I invented my_rev_post_order_compute ...
(pre_and_rev_post_order_compute doesn't compute an optimal
reverse post order).

Patch fixing the liveness below, untested sofar, apart from on
tree-ssa.exp where it seems to regress gcc.dg/tree-ssa/pr21086.c :/

As for the equiv sets - yes, that's known.  I wanted to investigate
at some point what happens if we instead record the SSA name we
registered the assert for (thus look up a chain of lattice values
instead of recording all relevant entries in a bitmap).  ISTR there
were some correctness issues, but if we restrict the chaining maybe
we cat get a good compromise here.

Richard.

Index: gcc/tree-vrp.c
===================================================================
--- gcc/tree-vrp.c      (revision 191289)
+++ gcc/tree-vrp.c      (working copy)
@@ -5439,7 +5439,6 @@ find_assert_locations_1 (basic_block bb,
 {
   gimple_stmt_iterator si;
   gimple last;
-  gimple phi;
   bool need_assert;
 
   need_assert = false;
@@ -5462,7 +5461,7 @@ find_assert_locations_1 (basic_block bb,
 
   /* Traverse all the statements in BB marking used names and looking
      for statements that may infer assertions for their used operands.  */
-  for (si = gsi_start_bb (bb); !gsi_end_p (si); gsi_next (&si))
+  for (si = gsi_last_bb (bb); !gsi_end_p (si); gsi_prev (&si))
     {
       gimple stmt;
       tree op;
@@ -5531,6 +5530,9 @@ find_assert_locations_1 (basic_block bb,
                }
            }
        }
+
+      FOR_EACH_SSA_TREE_OPERAND (op, stmt, i, SSA_OP_DEF)
+       RESET_BIT (live, SSA_NAME_VERSION (op));
     }
 
   /* Traverse all PHI nodes in BB marking used operands.  */
@@ -5538,7 +5540,11 @@ find_assert_locations_1 (basic_block bb,
     {
       use_operand_p arg_p;
       ssa_op_iter i;
-      phi = gsi_stmt (si);
+      gimple phi = gsi_stmt (si);
+      tree res = gimple_phi_result (phi);
+
+      if (virtual_operand_p (res))
+       continue;
 
       FOR_EACH_PHI_ARG (arg_p, phi, i, SSA_OP_USE)
        {
@@ -5546,6 +5552,8 @@ find_assert_locations_1 (basic_block bb,
          if (TREE_CODE (arg) == SSA_NAME)
            SET_BIT (live, SSA_NAME_VERSION (arg));
        }
+
+      RESET_BIT (live, SSA_NAME_VERSION (res));
     }
 
   return need_assert;

Reply via email to