On Fri, Jun 23, 2017 at 6:06 PM, Bill Schmidt
<wschm...@linux.vnet.ibm.com> wrote:
> Hi,
>
> Here's version 2 of the patch to fix the missed SLSR PHI opportunities,
> addressing Richard's comments. I've repeated regstrap and SPEC testing
> on powerpc64le-unknown-linux-gnu, again showing the patch as neutral
> with respect to performance. Is this ok for trunk?
Ok!
Thanks,
Richard.
> Thanks for the review!
>
> Bill
>
>
> [gcc]
>
> 2016-06-23 Bill Schmidt <wschm...@linux.vnet.ibm.com>
>
> * gimple-ssa-strength-reduction.c (uses_consumed_by_stmt): New
> function.
> (find_basis_for_candidate): Call uses_consumed_by_stmt rather than
> has_single_use.
> (slsr_process_phi): Likewise.
> (replace_uncond_cands_and_profitable_phis): Don't replace a
> multiply candidate with a stride of 1 (copy or cast).
> (phi_incr_cost): Call uses_consumed_by_stmt rather than
> has_single_use.
> (lowest_cost_path): Likewise.
> (total_savings): Likewise.
>
> [gcc/testsuite]
>
> 2016-06-23 Bill Schmidt <wschm...@linux.vnet.ibm.com>
>
> * gcc.dg/tree-ssa/slsr-35.c: Remove -fno-code-hoisting workaround.
> * gcc.dg/tree-ssa/slsr-36.c: Likewise.
>
>
> Index: gcc/gimple-ssa-strength-reduction.c
> ===================================================================
> --- gcc/gimple-ssa-strength-reduction.c (revision 249223)
> +++ gcc/gimple-ssa-strength-reduction.c (working copy)
> @@ -482,6 +482,36 @@ find_phi_def (tree base)
> return c->cand_num;
> }
>
> +/* Determine whether all uses of NAME are directly or indirectly
> + used by STMT. That is, we want to know whether if STMT goes
> + dead, the definition of NAME also goes dead. */
> +static bool
> +uses_consumed_by_stmt (tree name, gimple *stmt, unsigned recurse = 0)
> +{
> + gimple *use_stmt;
> + imm_use_iterator iter;
> + bool retval = true;
> +
> + FOR_EACH_IMM_USE_STMT (use_stmt, iter, name)
> + {
> + if (use_stmt == stmt || is_gimple_debug (use_stmt))
> + continue;
> +
> + if (!is_gimple_assign (use_stmt)
> + || !gimple_get_lhs (use_stmt)
> + || !is_gimple_reg (gimple_get_lhs (use_stmt))
> + || recurse >= 10
> + || !uses_consumed_by_stmt (gimple_get_lhs (use_stmt), stmt,
> + recurse + 1))
> + {
> + retval = false;
> + BREAK_FROM_IMM_USE_STMT (iter);
> + }
> + }
> +
> + return retval;
> +}
> +
> /* Helper routine for find_basis_for_candidate. May be called twice:
> once for the candidate's base expr, and optionally again either for
> the candidate's phi definition or for a CAND_REF's alternative base
> @@ -558,7 +588,8 @@ find_basis_for_candidate (slsr_cand_t c)
>
> /* If we found a hidden basis, estimate additional dead-code
> savings if the phi and its feeding statements can be removed. */
> - if (basis && has_single_use (gimple_phi_result
> (phi_cand->cand_stmt)))
> + tree feeding_var = gimple_phi_result (phi_cand->cand_stmt);
> + if (basis && uses_consumed_by_stmt (feeding_var, c->cand_stmt))
> c->dead_savings += phi_cand->dead_savings;
> }
> }
> @@ -789,7 +820,7 @@ slsr_process_phi (gphi *phi, bool speed)
>
> /* Gather potential dead code savings if the phi statement
> can be removed later on. */
> - if (has_single_use (arg))
> + if (uses_consumed_by_stmt (arg, phi))
> {
> if (gimple_code (arg_stmt) == GIMPLE_PHI)
> savings += arg_cand->dead_savings;
> @@ -2479,7 +2510,9 @@ replace_uncond_cands_and_profitable_phis (slsr_can
> {
> if (phi_dependent_cand_p (c))
> {
> - if (c->kind == CAND_MULT)
> + /* A multiply candidate with a stride of 1 is just an artifice
> + of a copy or cast; there is no value in replacing it. */
> + if (c->kind == CAND_MULT && wi::to_widest (c->stride) != 1)
> {
> /* A candidate dependent upon a phi will replace a multiply by
> a constant with an add, and will insert at most one add for
> @@ -2725,8 +2758,9 @@ phi_incr_cost (slsr_cand_t c, const widest_int &in
> if (gimple_code (arg_def) == GIMPLE_PHI)
> {
> int feeding_savings = 0;
> + tree feeding_var = gimple_phi_result (arg_def);
> cost += phi_incr_cost (c, incr, arg_def, &feeding_savings);
> - if (has_single_use (gimple_phi_result (arg_def)))
> + if (uses_consumed_by_stmt (feeding_var, phi))
> *savings += feeding_savings;
> }
> else
> @@ -2739,7 +2773,7 @@ phi_incr_cost (slsr_cand_t c, const widest_int &in
> tree basis_lhs = gimple_assign_lhs (basis->cand_stmt);
> tree lhs = gimple_assign_lhs (arg_cand->cand_stmt);
> cost += add_cost (true, TYPE_MODE (TREE_TYPE (basis_lhs)));
> - if (has_single_use (lhs))
> + if (uses_consumed_by_stmt (lhs, phi))
> *savings += stmt_cost (arg_cand->cand_stmt, true);
> }
> }
> @@ -2816,7 +2850,7 @@ lowest_cost_path (int cost_in, int repl_savings, s
> gimple *phi = lookup_cand (c->def_phi)->cand_stmt;
> local_cost += phi_incr_cost (c, incr, phi, &savings);
>
> - if (has_single_use (gimple_phi_result (phi)))
> + if (uses_consumed_by_stmt (gimple_phi_result (phi), c->cand_stmt))
> local_cost -= savings;
> }
>
> @@ -2860,7 +2894,7 @@ total_savings (int repl_savings, slsr_cand_t c, co
> gimple *phi = lookup_cand (c->def_phi)->cand_stmt;
> savings -= phi_incr_cost (c, incr, phi, &phi_savings);
>
> - if (has_single_use (gimple_phi_result (phi)))
> + if (uses_consumed_by_stmt (gimple_phi_result (phi), c->cand_stmt))
> savings += phi_savings;
> }
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-35.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-35.c (revision 249223)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-35.c (working copy)
> @@ -3,7 +3,7 @@
> phi has an argument which is a parameter. */
>
> /* { dg-do compile } */
> -/* { dg-options "-O3 -fno-code-hoisting -fdump-tree-optimized" } */
> +/* { dg-options "-O3 -fdump-tree-optimized" } */
>
> int
> f (int c, int i)
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-36.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-36.c (revision 249223)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-36.c (working copy)
> @@ -3,7 +3,7 @@
> phi has an argument which is a parameter. */
>
> /* { dg-do compile } */
> -/* { dg-options "-O3 -fno-code-hoisting -fdump-tree-optimized" } */
> +/* { dg-options "-O3 -fdump-tree-optimized" } */
>
> int
> f (int s, int c, int i)
>
