Re: [PATCH] Fix thinko in SLSR that caused x86 bootstrap failure

[Richard Biener <richard dot guenther at gmail dot com>]

On Sun, May 5, 2013 at 10:45 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> When creating a phi-adjustment along an incoming edge, where the related
> argument should use the "hidden basis" directly, I created a situation
> where we double-count the subsequent adjustment based on the candidate's
> index.  The candidate's index shouldn't be taken into account at all for
> the edge adjustment; it will be handled in relation to the created phi
> basis.  This patch fixes that thinko.
>
> I disabled processing of conditional candidates (and associated tests)
> on Friday night to allow x86 targets to bootstrap cleanly.  This patch
> also reinstates the disabled code.
>
> Bootstrapped and tested on powerpc-unknown-linux-gnu and
> i686-pc-linux-gnu with no new regressions.  Ok for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Bill
>
>
>
> gcc:
>
> 2013-05-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * gimple-ssa-strength-reduction.c (slsr_process_phi): Re-enable.
>         (find_candidates_in_block): Re-enable slsr_process_phi.
>         (create_phi_basis): Fix double counting of candidate adjustment.
>
> gcc/testsuite:
>
> 2013-05-05  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * gcc.dg/tree-ssa/slsr-32.c: Re-enable.
>         * gcc.dg/tree-ssa/slsr-33.c: Likewise.
>         * gcc.dg/tree-ssa/slsr-34.c: Likewise.
>         * gcc.dg/tree-ssa/slsr-35.c: Likewise.
>         * gcc.dg/tree-ssa/slsr-36.c: Likewise.
>         * gcc.dg/tree-ssa/slsr-37.c: Likewise.
>         * gcc.dg/tree-ssa/slsr-38.c: Likewise.
>
>
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-32.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-32.c     (revision 198611)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-32.c     (working copy)
> @@ -3,7 +3,6 @@
>
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-optimized" } */
> -/* { dg-skip-if "" { *-*-* } } */
>
>  int
>  f (int s, int c, int i)
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-33.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-33.c     (revision 198611)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-33.c     (working copy)
> @@ -3,7 +3,6 @@
>
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-optimized" } */
> -/* { dg-skip-if "" { *-*-* } } */
>
>  int
>  f (int c, int i)
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c     (revision 198611)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-34.c     (working copy)
> @@ -3,7 +3,6 @@
>
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-optimized" } */
> -/* { dg-skip-if "" { *-*-* } } */
>
>  extern void
>  g (void);
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-35.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-35.c     (revision 198611)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-35.c     (working copy)
> @@ -4,7 +4,6 @@
>
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-optimized" } */
> -/* { dg-skip-if "" { *-*-* } } */
>
>  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 198611)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-36.c     (working copy)
> @@ -4,7 +4,6 @@
>
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-optimized" } */
> -/* { dg-skip-if "" { *-*-* } } */
>
>  int
>  f (int s, int c, int i)
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-37.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-37.c     (revision 198611)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-37.c     (working copy)
> @@ -4,7 +4,6 @@
>
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-optimized" } */
> -/* { dg-skip-if "" { *-*-* } } */
>
>  int
>  f (int s, int c, int i)
> Index: gcc/testsuite/gcc.dg/tree-ssa/slsr-38.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/slsr-38.c     (revision 198611)
> +++ gcc/testsuite/gcc.dg/tree-ssa/slsr-38.c     (working copy)
> @@ -4,7 +4,6 @@
>
>  /* { dg-do compile } */
>  /* { dg-options "-O3 -fdump-tree-optimized" } */
> -/* { dg-skip-if "" { *-*-* } } */
>
>  int
>  f (int c, int i)
> Index: gcc/gimple-ssa-strength-reduction.c
> ===================================================================
> --- gcc/gimple-ssa-strength-reduction.c (revision 198611)
> +++ gcc/gimple-ssa-strength-reduction.c (working copy)
> @@ -657,9 +657,6 @@ add_cand_for_stmt (gimple gs, slsr_cand_t c)
>    *slot = c;
>  }
>
> -// FORNOW: Disable conditional candidate processing until bootstrap
> -// issue can be sorted out for i686-pc-linux-gnu.
> -#if 0
>  /* Given PHI which contains a phi statement, determine whether it
>     satisfies all the requirements of a phi candidate.  If so, create
>     a candidate.  Note that a CAND_PHI never has a basis itself, but
> @@ -750,7 +747,6 @@ slsr_process_phi (gimple phi, bool speed)
>    /* Add the candidate to the statement-candidate mapping.  */
>    add_cand_for_stmt (phi, c);
>  }
> -#endif
>
>  /* Look for the following pattern:
>
> @@ -1523,12 +1519,8 @@ find_candidates_in_block (struct dom_walk_data *wa
>    bool speed = optimize_bb_for_speed_p (bb);
>    gimple_stmt_iterator gsi;
>
> -// FORNOW: Disable conditional candidate processing until bootstrap
> -// issue can be sorted out for i686-pc-linux-gnu.
> -#if 0
>    for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>      slsr_process_phi (gsi_stmt (gsi), speed);
> -#endif
>
>    for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
>      {
> @@ -2098,7 +2090,7 @@ create_phi_basis (slsr_cand_t c, gimple from_phi,
>           feeding_def = gimple_assign_lhs (basis->cand_stmt);
>         else
>           {
> -           double_int incr = c->index - basis->index;
> +           double_int incr = -basis->index;
>             feeding_def = create_add_on_incoming_edge (c, basis_name, incr,
>                                                        e, loc, known_stride);
>           }
>
>