On Tue, 15 Apr 2014, Jakub Jelinek wrote:
> Hi!
>
> The (admittedly ugly) reassoc stmt positioning stuff requires that
> we maintain uids in ascending order within each bb (equal uid for several
> adjacent stmts is ok), including debug stmts.
> We assign those initially, and for stmts we add we make sure to copy the
> uid from some adjacent insn.
>
> But, as the following testcase shows, we don't take into account that
> gsi_remove can add debug stmts, and those don't have uid set.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
> ok for trunk/4.9.1?
Ok.
Thanks,
Richard.
> 2014-04-15 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/60844
> * tree-ssa-reassoc.c (reassoc_remove_stmt): New function.
> (propagate_op_to_single_use, remove_visited_stmt_chain,
> linearize_expr, repropagate_negates, reassociate_bb): Use it
> instead of gsi_remove.
>
> * gcc.dg/pr60844.c: New test.
>
> --- gcc/tree-ssa-reassoc.c.jj 2014-03-13 10:38:09.000000000 +0100
> +++ gcc/tree-ssa-reassoc.c 2014-04-15 13:59:14.511383249 +0200
> @@ -221,6 +221,35 @@ static struct pointer_map_t *operand_ran
> static long get_rank (tree);
> static bool reassoc_stmt_dominates_stmt_p (gimple, gimple);
>
> +/* Wrapper around gsi_remove, which adjusts gimple_uid of debug stmts
> + possibly added by gsi_remove. */
> +
> +bool
> +reassoc_remove_stmt (gimple_stmt_iterator *gsi)
> +{
> + gimple stmt = gsi_stmt (*gsi);
> +
> + if (!MAY_HAVE_DEBUG_STMTS || gimple_code (stmt) == GIMPLE_PHI)
> + return gsi_remove (gsi, true);
> +
> + gimple_stmt_iterator prev = *gsi;
> + gsi_prev (&prev);
> + unsigned uid = gimple_uid (stmt);
> + basic_block bb = gimple_bb (stmt);
> + bool ret = gsi_remove (gsi, true);
> + if (!gsi_end_p (prev))
> + gsi_next (&prev);
> + else
> + prev = gsi_start_bb (bb);
> + gimple end_stmt = gsi_stmt (*gsi);
> + while ((stmt = gsi_stmt (prev)) != end_stmt)
> + {
> + gcc_assert (stmt && is_gimple_debug (stmt) && gimple_uid (stmt) == 0);
> + gimple_set_uid (stmt, uid);
> + gsi_next (&prev);
> + }
> + return ret;
> +}
>
> /* Bias amount for loop-carried phis. We want this to be larger than
> the depth of any reassociation tree we can see, but not larger than
> @@ -1123,7 +1152,7 @@ propagate_op_to_single_use (tree op, gim
> update_stmt (use_stmt);
> gsi = gsi_for_stmt (stmt);
> unlink_stmt_vdef (stmt);
> - gsi_remove (&gsi, true);
> + reassoc_remove_stmt (&gsi);
> release_defs (stmt);
> }
>
> @@ -3072,7 +3101,7 @@ remove_visited_stmt_chain (tree var)
> {
> var = gimple_assign_rhs1 (stmt);
> gsi = gsi_for_stmt (stmt);
> - gsi_remove (&gsi, true);
> + reassoc_remove_stmt (&gsi);
> release_defs (stmt);
> }
> else
> @@ -3494,7 +3523,7 @@ linearize_expr (gimple stmt)
> update_stmt (stmt);
>
> gsi = gsi_for_stmt (oldbinrhs);
> - gsi_remove (&gsi, true);
> + reassoc_remove_stmt (&gsi);
> release_defs (oldbinrhs);
>
> gimple_set_visited (stmt, true);
> @@ -3896,7 +3925,7 @@ repropagate_negates (void)
> gimple_assign_set_rhs_with_ops (&gsi2, NEGATE_EXPR, x, NULL);
> user = gsi_stmt (gsi2);
> update_stmt (user);
> - gsi_remove (&gsi, true);
> + reassoc_remove_stmt (&gsi);
> release_defs (feed);
> plus_negates.safe_push (gimple_assign_lhs (user));
> }
> @@ -4413,7 +4442,7 @@ reassociate_bb (basic_block bb)
> reassociations. */
> if (has_zero_uses (gimple_get_lhs (stmt)))
> {
> - gsi_remove (&gsi, true);
> + reassoc_remove_stmt (&gsi);
> release_defs (stmt);
> /* We might end up removing the last stmt above which
> places the iterator to the end of the sequence.
> --- gcc/testsuite/gcc.dg/pr60844.c.jj 2014-04-15 14:01:27.561689401 +0200
> +++ gcc/testsuite/gcc.dg/pr60844.c 2014-04-15 14:01:10.000000000 +0200
> @@ -0,0 +1,16 @@
> +/* PR tree-optimization/60844 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +/* { dg-additional-options "-mtune=atom" { target { i?86-*-* x86_64-*-* } }
> } */
> +
> +void
> +foo (int *x, int y, int z)
> +{
> + int b, c = x[0], d = x[1];
> + for (b = 0; b < 1; b++)
> + {
> + int e = (y ? 1 : 0) | (d ? 2 : 0) | (z ? 1 : 0);
> + e |= (c ? 2 : 0) | ((1 >> b) ? 1 : 0);
> + x[2 + b] = e;
> + }
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
