On Tue, May 7, 2013 at 12:16 AM, Bill Schmidt
<wschm...@linux.vnet.ibm.com> wrote:
> This fixes the following bug:
>
> On Mon, 2013-05-06 at 21:25 +0200, Jakub Jelinek wrote:
>> On Sun, May 05, 2013 at 03:45:17PM -0500, Bill Schmidt wrote:
>> > 2013-05-05 Bill Schmidt <wschm...@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.
>>
>> This broke gcc.dg/pr33017.c testcase on i?86/x86_64 -m32.
>> ./cc1 -O2 -ftree-vectorize -m32 -mno-sse pr33017.c
>> difference between r19862{6,7} is:
>> --- pr33017.s1 2013-05-06 21:22:03.786745422 +0200
>> +++ pr33017.s2 2013-05-06 21:22:16.844673015 +0200
>> @@ -32,9 +32,9 @@ foo:
>> movb $87, var.1373+2(%eax)
>> je .L10
>> cmpl $3, %edx
>> - movb $87, var.1373+3(%eax)
>> + movb $87, var.1373+2(%eax,%eax)
>> jne .L11
>> - movb $87, var.1373+4(%eax)
>> + movb $87, var.1373+2(%eax,%eax,2)
>> movl $3, %ebp
>> movl $61, 28(%esp)
>> .L3:
>>
>> Jakub
>>
>
> The pattern we seek for replacing a conditional candidate can be
> confused when a CAND_ADD with an index of 1 has its operands commuted in
> the analysis. (For x = y + z, we may record two CAND_ADDs, one for
> either ordering of the addends.) For a candidate with reversed addends
> in the analysis, don't attempt to find a hidden basis. Otherwise we
> make an invalid replacement later on.
>
> Verified this fixes the reported bug. Bootstrapped and tested on
> powerpc64-unknown-linux-gnu with no new regressions. Ok for trunk?
>
> Thanks,
> Bill
>
>
> 2013-05-06 Bill Schmidt <wschm...@linux.vnet.ibm.com>
>
> * gimple-ssa-strength-reduction.c (find_phi_def): Don't record a
> phi def as possibly hiding a basis for a CAND_ADD whose operands
> have been commuted in the analysis.
> (alloc_cand_and_find_basis): Add parms to call to find_phi_def.
>
>
> Index: gcc/gimple-ssa-strength-reduction.c
> ===================================================================
> --- gcc/gimple-ssa-strength-reduction.c (revision 198627)
> +++ gcc/gimple-ssa-strength-reduction.c (working copy)
> @@ -413,15 +413,29 @@ 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. */
> + 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. */
>
> static cand_idx
> -find_phi_def (tree base)
> +find_phi_def (gimple gs, enum cand_kind kind, tree base,
> + double_int index, tree stride)
> {
> slsr_cand_t c;
>
> if (TREE_CODE (base) != SSA_NAME)
> return 0;
> +
> + /* Check for commutativity condition. */
Please adjust the comment - the test checks more and bails out. So
the comment should say why.
Ok with that change.
Thanks,
Richard.
> + 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);
>
> @@ -565,7 +579,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 (base);
> + c->def_phi = find_phi_def (gs, kind, base, index, stride);
> c->dead_savings = savings;
>
> cand_vec.safe_push (c);
>
>
