Patchwork Restrict conditional SLSR candidates to CAND_MULTs

login
register
mail settings
Submitter William J. Schmidt
Date May 7, 2013, 6:12 p.m.
Message ID <1367950343.4938.34.camel@oc8801110288.ibm.com>
Download mbox | patch
Permalink /patch/242433/
State New
Headers show

Comments

William J. Schmidt - May 7, 2013, 6:12 p.m.
The intent of conditional candidate processing in SLSR was always to
apply it only to CAND_MULT candidates [(base + index) * stride].
However, I neglected to actually enforce this, leading to wrong code
generated for CAND_ADD candidates.  This patch adds the restriction.

My previous "fix" wasn't sufficient; at the time I thought we were
handling CAND_ADD candidates improperly, when in reality we shouldn't
have been handling them at all.  So this patch also backs out that
change, which becomes dead code with the CAND_MULT restriction.

I've verified this fixes the pr33017.c and vect-28.c tests.  I haven't
yet reproduced the remaining Fortran problems, but I am hopeful that
this will fix those as well.  If you see these have been fixed, please
let me know; otherwise I will work on reproducing them.

Bootstrapped and tested on powerpc64-unknown-linux-gnu, -m32/-m64, with
no new regressions.  Ok for trunk?

Thanks,
Bill


2013-05-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* gimple-ssa-strength-reduction.c (find_phi_def): Revert former	"fix."
	(alloc_cand_and_find_basis): Restrict conditional candidate
	processing to CAND_MULTs.
Richard Guenther - May 8, 2013, 8:21 a.m.
On Tue, 7 May 2013, Bill Schmidt wrote:

> The intent of conditional candidate processing in SLSR was always to
> apply it only to CAND_MULT candidates [(base + index) * stride].
> However, I neglected to actually enforce this, leading to wrong code
> generated for CAND_ADD candidates.  This patch adds the restriction.
> 
> My previous "fix" wasn't sufficient; at the time I thought we were
> handling CAND_ADD candidates improperly, when in reality we shouldn't
> have been handling them at all.  So this patch also backs out that
> change, which becomes dead code with the CAND_MULT restriction.
> 
> I've verified this fixes the pr33017.c and vect-28.c tests.  I haven't
> yet reproduced the remaining Fortran problems, but I am hopeful that
> this will fix those as well.  If you see these have been fixed, please
> let me know; otherwise I will work on reproducing them.
> 
> Bootstrapped and tested on powerpc64-unknown-linux-gnu, -m32/-m64, with
> no new regressions.  Ok for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Bill
> 
> 
> 2013-05-07  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> 
> 	* gimple-ssa-strength-reduction.c (find_phi_def): Revert former	"fix."
> 	(alloc_cand_and_find_basis): Restrict conditional candidate
> 	processing to CAND_MULTs.
> 
> 
> Index: gcc/gimple-ssa-strength-reduction.c
> ===================================================================
> --- gcc/gimple-ssa-strength-reduction.c	(revision 198682)
> +++ gcc/gimple-ssa-strength-reduction.c	(working copy)
> @@ -415,32 +415,16 @@ cand_chain_hasher::equal (const value_type *chain1
>  static hash_table <cand_chain_hasher> base_cand_map;
>  
>  /* Look in the candidate table for a CAND_PHI that defines BASE and
> -   return it if found; otherwise return NULL.  GS is the candidate
> -   statement with BASE, INDEX, and STRIDE.  If GS is a CAND_ADD with
> -   an index of 1 and an SSA name for STRIDE, we must be careful that
> -   we haven't commuted the operands for this candidate.  STRIDE must
> -   correspond to the second addend of GS for the eventual transformation
> -   to be legal.  If not, return NULL.  */
> +   return it if found; otherwise return NULL.  */
>  
>  static cand_idx
> -find_phi_def (gimple gs, enum cand_kind kind, tree base,
> -              double_int index, tree stride)
> +find_phi_def (tree base)
>  {
>    slsr_cand_t c;
>  
>    if (TREE_CODE (base) != SSA_NAME)
>      return 0;
>  
> -  /* If we've commuted the operands (so "y + z" is represented as
> -     "z + (1 * y)"), we don't have the pattern we're looking for.
> -     Bail out to avoid doing a wrong replacement downstream.  */
> -  if (kind == CAND_ADD
> -      && index.is_one ()
> -      && TREE_CODE (stride) == SSA_NAME
> -      && gimple_assign_rhs_code (gs) == PLUS_EXPR
> -      && stride != gimple_assign_rhs2 (gs))
> -    return 0;
> -  
>    c = base_cand_from_table (base);
>  
>    if (!c || c->kind != CAND_PHI)
> @@ -583,7 +567,7 @@ alloc_cand_and_find_basis (enum cand_kind kind, gi
>    c->next_interp = 0;
>    c->dependent = 0;
>    c->sibling = 0;
> -  c->def_phi = find_phi_def (gs, kind, base, index, stride);
> +  c->def_phi = kind == CAND_MULT ? find_phi_def (base) : 0;
>    c->dead_savings = savings;
>  
>    cand_vec.safe_push (c);
> 
> 
>

Patch

Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c	(revision 198682)
+++ gcc/gimple-ssa-strength-reduction.c	(working copy)
@@ -415,32 +415,16 @@  cand_chain_hasher::equal (const value_type *chain1
 static hash_table <cand_chain_hasher> base_cand_map;
 
 /* Look in the candidate table for a CAND_PHI that defines BASE and
-   return it if found; otherwise return NULL.  GS is the candidate
-   statement with BASE, INDEX, and STRIDE.  If GS is a CAND_ADD with
-   an index of 1 and an SSA name for STRIDE, we must be careful that
-   we haven't commuted the operands for this candidate.  STRIDE must
-   correspond to the second addend of GS for the eventual transformation
-   to be legal.  If not, return NULL.  */
+   return it if found; otherwise return NULL.  */
 
 static cand_idx
-find_phi_def (gimple gs, enum cand_kind kind, tree base,
-              double_int index, tree stride)
+find_phi_def (tree base)
 {
   slsr_cand_t c;
 
   if (TREE_CODE (base) != SSA_NAME)
     return 0;
 
-  /* If we've commuted the operands (so "y + z" is represented as
-     "z + (1 * y)"), we don't have the pattern we're looking for.
-     Bail out to avoid doing a wrong replacement downstream.  */
-  if (kind == CAND_ADD
-      && index.is_one ()
-      && TREE_CODE (stride) == SSA_NAME
-      && gimple_assign_rhs_code (gs) == PLUS_EXPR
-      && stride != gimple_assign_rhs2 (gs))
-    return 0;
-  
   c = base_cand_from_table (base);
 
   if (!c || c->kind != CAND_PHI)
@@ -583,7 +567,7 @@  alloc_cand_and_find_basis (enum cand_kind kind, gi
   c->next_interp = 0;
   c->dependent = 0;
   c->sibling = 0;
-  c->def_phi = find_phi_def (gs, kind, base, index, stride);
+  c->def_phi = kind == CAND_MULT ? find_phi_def (base) : 0;
   c->dead_savings = savings;
 
   cand_vec.safe_push (c);