Re: [PATCH] PR tree-optimization/68413 : Only check for integer cond reduction on analysis stage

[Richard Biener <richard dot guenther at gmail dot com>]

On Fri, Nov 20, 2015 at 1:33 PM, Alan Hayward <alan.hayward@arm.com> wrote:
>
>
> On 20/11/2015 11:00, "Richard Biener" <richard.guenther@gmail.com> wrote:
>
>>On Fri, Nov 20, 2015 at 10:24 AM, Alan Hayward <alan.hayward@arm.com>
>>wrote:
>>> When vectorising a integer induction condition reduction,
>>> is_nonwrapping_integer_induction ends up with different values for base
>>> during the analysis and build phases. In the first it is an INTEGER_CST,
>>> in the second the loop has been vectorised out and the base is now a
>>> variable.
>>>
>>> This results in the analysis and build stage detecting the
>>> STMT_VINFO_VEC_REDUCTION_TYPE as different types.
>>>
>>> The easiest way to fix this is to only check for integer induction
>>> conditions on the analysis stage.
>>
>>I don't like this.  For the evolution part we have added
>>STMT_VINFO_LOOP_PHI_EVOLUTION_PART.  If you now need
>>the original initial value as well then just save it.
>>
>>Or if you really want to go with the hack then please do not call
>>is_nonwrapping_integer_induction with vec_stmt != NULL but
>>initialize cond_expr_is_nonwrapping_integer_induction from
>>STMT_VINFO_VEC_REDUCTION_TYPE (stmt_info)
>>
>>The hack also lacks a comment.
>>
>
> Ok. I've gone for a combination of both:
>
> I now cache the base in STMT_VINFO_LOOP_PHI_EVOLUTION_BASE.
>
> I've removed the vec_stmt != NULL checks.
>
> I've moved the call to is_nonwrapping_integer_induction until after the
> vect_is_simple_reduction check. I never liked that I had
> is_nonwrapping_integer_induction early in the function, and think this
> looks better.

It looks better but the comment for loop_phi_evolution_base is wrong.
The value is _not_ "correct" after prologue peeling (unless you
update it there to a non-constant expr).  It is conservatively "correct"
for is_nonwrapping_integer_induction though.  Which is why I'd
probably rename it to STMT_VINFO_LOOP_PHI_EVOLUTION_BASE_UNCHANGED
to reflect this (and similar to STMT_VINFO_NITERS_UNCHANGED).

Ok with that change.

Thanks,
Richard.

>
> Alan.
>