Patchwork Fix thinko in SLSR that caused x86 bootstrap failure

login
register
mail settings
Submitter William J. Schmidt
Date May 5, 2013, 8:45 p.m.
Message ID <1367786717.11638.58.camel@gnopaine>
Download mbox | patch
Permalink /patch/241526/
State New
Headers show

Comments

William J. Schmidt - May 5, 2013, 8:45 p.m.
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?

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.
Richard Guenther - May 6, 2013, 11:16 a.m.
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);
>           }
>
>

Patch

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);
 	  }