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 <wschm...@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);
>
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend
