| Message ID | 1367950343.4938.34.camel@oc8801110288.ibm.com |
|---|---|
| State | New |
| Headers | show |
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); > > >
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);