On Tue, May 8, 2012 at 8:33 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> The attached testcase loops endlessly, using more and more memory.
> The problem is that the prev stmt iterator sometimes references stmts that
> remove_prop_source_from_use decides to remove, and since Michael's
> gimple seq changes that seems to be fatal.
>
> Fixed by not keeping an iterator, but instead marking stmts that don't need
> revisiting and restarting with first stmt that needs revisiting. This
> assumes that new stmts will have uid 0, but I believe that is the case.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Using the stmt UID looks odd as you are using it as flag - why not use
one of the two stmt flags available to passes?
Btw, the other possibility would be to simply change the various combiners
to require them to update the iterator to point at the first statement that
they have inserted. I have some TLC ideas in this area, let's see if I can
get to them.
Ok with using gimple_set_plf and friends.
Thanks,
Richard.
> 2012-05-08 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/53226
> * tree-ssa-forwprop.c (ssa_forward_propagate_and_combine): Remove
> prev and prev_initialized vars, gimple_set_uid (stmt, 0) before
> processing it and gimple_set_uid (stmt, 1) if it doesn't need to be
> revisited, look for earliest stmt with uid 0 if something changed.
>
> * gcc.c-torture/compile/pr53226.c: New test.
>
> --- gcc/tree-ssa-forwprop.c.jj 2012-05-03 08:35:52.000000000 +0200
> +++ gcc/tree-ssa-forwprop.c 2012-05-08 18:10:19.662061709 +0200
> @@ -2677,8 +2677,7 @@ ssa_forward_propagate_and_combine (void)
>
> FOR_EACH_BB (bb)
> {
> - gimple_stmt_iterator gsi, prev;
> - bool prev_initialized;
> + gimple_stmt_iterator gsi;
>
> /* Apply forward propagation to all stmts in the basic-block.
> Note we update GSI within the loop as necessary. */
> @@ -2771,12 +2770,14 @@ ssa_forward_propagate_and_combine (void)
>
> /* Combine stmts with the stmts defining their operands.
> Note we update GSI within the loop as necessary. */
> - prev_initialized = false;
> for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
> {
> gimple stmt = gsi_stmt (gsi);
> bool changed = false;
>
> + /* Mark stmt as potentially needing revisiting. */
> + gimple_set_uid (stmt, 0);
> +
> switch (gimple_code (stmt))
> {
> case GIMPLE_ASSIGN:
> @@ -2856,18 +2857,18 @@ ssa_forward_propagate_and_combine (void)
> {
> /* If the stmt changed then re-visit it and the statements
> inserted before it. */
> - if (!prev_initialized)
> + for (; !gsi_end_p (gsi); gsi_prev (&gsi))
> + if (gimple_uid (gsi_stmt (gsi)))
> + break;
> + if (gsi_end_p (gsi))
> gsi = gsi_start_bb (bb);
> else
> - {
> - gsi = prev;
> - gsi_next (&gsi);
> - }
> + gsi_next (&gsi);
> }
> else
> {
> - prev = gsi;
> - prev_initialized = true;
> + /* Stmt no longer needs to be revisited. */
> + gimple_set_uid (stmt, 1);
> gsi_next (&gsi);
> }
> }
> --- gcc/testsuite/gcc.c-torture/compile/pr53226.c.jj 2012-05-08
> 18:07:40.007000510 +0200
> +++ gcc/testsuite/gcc.c-torture/compile/pr53226.c 2012-05-08
> 18:07:35.071029578 +0200
> @@ -0,0 +1,13 @@
> +/* PR tree-optimization/53226 */
> +
> +void
> +foo (unsigned long *x, char y, char z)
> +{
> + int i;
> + for (i = y; i < z; ++i)
> + {
> + unsigned long a = ((unsigned char) i) & 63UL;
> + unsigned long b = 1ULL << a;
> + *x |= b;
> + }
> +}
>
> Jakub
