On Thu, 13 Jul 2017, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, for aggregate copies we fail to verify there
> are no loads in between the PHI and the aggregate copy that could alias with
> the
> lhs of the copy, which is needed, because we want to hoist the aggregate
> copy onto predecessor edges of the bb with the PHI.
>
> The following patch implements that, bootstrapped/regtested on x86_64-linux
> and i686-linux, ok for trunk and after a while 7.x?
>
> 2017-07-13 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/81365
> * tree-ssa-phiprop.c (propagate_with_phi): When considering hoisting
> aggregate moves onto bb predecessor edges, make sure there are no
> loads that could alias the lhs in between the start of bb and the
> loads from *phi. If there are any debug stmts that could alias,
> reset them.
>
> * g++.dg/torture/pr81365.C: New test.
>
> --- gcc/tree-ssa-phiprop.c.jj 2017-05-22 10:50:11.000000000 +0200
> +++ gcc/tree-ssa-phiprop.c 2017-07-11 16:52:41.012340615 +0200
> @@ -327,7 +327,7 @@ propagate_with_phi (basic_block bb, gphi
> if (!dominated_by_p (CDI_POST_DOMINATORS,
> bb, gimple_bb (use_stmt)))
> continue;
> -
> +
> /* Check whether this is a load of *ptr. */
> if (!(is_gimple_assign (use_stmt)
> && gimple_assign_rhs_code (use_stmt) == MEM_REF
> @@ -356,6 +356,9 @@ propagate_with_phi (basic_block bb, gphi
> insert aggregate copies on the edges instead. */
> if (!is_gimple_reg_type (TREE_TYPE (TREE_TYPE (ptr))))
> {
> + if (!gimple_vdef (use_stmt))
> + goto next;
> +
> /* As we replicate the lhs on each incoming edge all
> used SSA names have to be available there. */
> if (! for_each_index (gimple_assign_lhs_ptr (use_stmt),
> @@ -363,6 +366,51 @@ propagate_with_phi (basic_block bb, gphi
> get_immediate_dominator (CDI_DOMINATORS,
> gimple_bb (phi))))
> goto next;
> +
> + gimple *vuse_stmt;
> + imm_use_iterator vui;
> + use_operand_p vuse_p;
> + bool debug_use_seen = false;
> + /* In order to move the aggregate copies earlier, make sure
> + there are no statements that could read from memory
> + aliasing the lhs in between the start of bb and use_stmt.
> + As we require use_stmt to have a VDEF above, loads after
> + use_stmt will use a different virtual SSA_NAME. */
> + FOR_EACH_IMM_USE_FAST (vuse_p, vui, vuse)
> + {
> + vuse_stmt = USE_STMT (vuse_p);
> + if (vuse_stmt == use_stmt)
> + continue;
> + if (!dominated_by_p (CDI_DOMINATORS,
> + gimple_bb (vuse_stmt), bb))
> + continue;
> + if (ref_maybe_used_by_stmt_p (vuse_stmt,
> + gimple_assign_lhs (use_stmt)))
> + {
> + if (is_gimple_debug (vuse_stmt))
debug stmts do not have virtual operands so their handling is not
necessary here.
Ok with that change.
Thanks,
Richard.
> + debug_use_seen = true;
> + else
> + goto next;
> + }
> + }
> + /* Debug stmt uses should not prevent the transformation, but
> + if we saw any, reset those debug stmts. */
> + if (debug_use_seen)
> + FOR_EACH_IMM_USE_STMT (vuse_stmt, vui, vuse)
> + {
> + if (!is_gimple_debug (vuse_stmt))
> + continue;
> + if (!dominated_by_p (CDI_DOMINATORS,
> + gimple_bb (vuse_stmt), bb))
> + continue;
> + if (ref_maybe_used_by_stmt_p (vuse_stmt,
> + gimple_assign_lhs (use_stmt)))
> + {
> + gimple_debug_bind_reset_value (vuse_stmt);
> + update_stmt (vuse_stmt);
> + }
> + }
> +
> phiprop_insert_phi (bb, phi, use_stmt, phivn, n);
>
> /* Remove old stmt. The phi is taken care of by DCE. */
> --- gcc/testsuite/g++.dg/torture/pr81365.C.jj 2017-07-11 17:07:11.107130111
> +0200
> +++ gcc/testsuite/g++.dg/torture/pr81365.C 2017-07-11 17:06:52.000000000
> +0200
> @@ -0,0 +1,39 @@
> +// PR tree-optimization/81365
> +// { dg-do run }
> +
> +struct A { unsigned a; };
> +
> +struct B {
> + B (const A *x)
> + {
> + __builtin_memcpy (b, x, 3 * sizeof (A));
> + __builtin_memcpy (c, x + 3, sizeof (A));
> + __builtin_memset (c + 1, 0, sizeof (A));
> + }
> + bool
> + foo (unsigned x)
> + {
> + A *it = c;
> + if (it->a == x || (++it)->a == x)
> + {
> + A t(b[0]);
> + b[0] = *it;
> + *it = t;
> + return true;
> + }
> + return false;
> + }
> + A b[3];
> + A c[2];
> +};
> +
> +int
> +main ()
> +{
> + A x[] = { 4, 8, 12, 18 };
> + B y(x);
> + if (!y.foo (18))
> + __builtin_abort ();
> + if (!y.foo (4))
> + __builtin_abort ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
