On Fri, Nov 20, 2015 at 1:33 PM, Alan Hayward <alan.hayw...@arm.com> wrote: > > > On 20/11/2015 11:00, "Richard Biener" <richard.guent...@gmail.com> wrote: > >>On Fri, Nov 20, 2015 at 10:24 AM, Alan Hayward <alan.hayw...@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. >