Re: [PATCH][2/3] Vectorize inductions that are live after the loop

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

On Thu, Jun 2, 2016 at 6:11 PM, Alan Hayward <alan.hayward@arm.com> wrote:
>
>
> On 01/06/2016 10:51, "Richard Biener" <richard.guenther@gmail.com> wrote:
>
>>On Wed, Jun 1, 2016 at 10:46 AM, Alan Hayward <alan.hayward@arm.com>
>>wrote:
>>>
>>>
>>> On 30/05/2016 14:22, "Richard Biener" <richard.guenther@gmail.com>
>>>wrote:
>>>
>>>>On Fri, May 27, 2016 at 5:12 PM, Alan Hayward <alan.hayward@arm.com>
>>>>wrote:
>>>>>
>>>>> On 27/05/2016 12:41, "Richard Biener" <richard.guenther@gmail.com>
>>>>>wrote:
>>>>>
>>>>>>On Fri, May 27, 2016 at 11:09 AM, Alan Hayward <alan.hayward@arm.com>
>>>>>>wrote:
>>>
>>>>>
>>>>>>
>>>>>>The rest of the changes look ok to me.
>>>>>
>>>>> Does that include PATCH 1/3  ?
>>>>
>>>>I don't like how 1/3 ends up looking :/  So what was the alternative
>>>>again?
>>>>I looked into 1/3 and what it takes to remove the 'stmt' argument and
>>>>instead pass in a vect_def_type.  It's a bit twisted and just adding
>>>>another
>>>>argument (the loop_vinfo) doesn't help things here.
>>>>
>>>>So - instead of 1/3 you might want to split out a function
>>>>
>>>>tree
>>>>vect_get_vec_def_for_operand_1 (gimple *def_stmt, enum vect_def_type
>>>>dt, tree vectype)
>>>>{
>>>>  switch (dt)
>>>>    {
>>>>...
>>>>    }
>>>>}
>>>>
>>>>and for constant/external force vectype != NULL.
>>>
>>> Iâm still a little confused as to exactly what you want here.
>>>
>>> From your two comments I think you wanted me to completely remove the
>>> boolean type check and the vect_init_vector call. But if I remove that
>>> then other code paths will break.
>>>
>>> However, Iâve just realised that in vectorized_live_operation I already
>>> have the def stmt and I can easily get hold of dt from
>>>STMT_VINFO_DEF_TYPE.
>>> Which means I can call vect_get_vec_def_for_operand_1 from
>>> vectorized_live_operation.
>>>
>>> Iâve put together a version where I have:
>>>
>>> tree
>>> vect_get_vec_def_for_operand_1 (gimple *def_stmt, enum vect_def_type dt)
>>> {
>>>
>>>  switch (dt)
>>>  {
>>>    case vect_internal_def || vect_external_def:
>>>      gcc_unreachable ()
>>>
>>>    .. code for for all other cases..
>>>  }
>>> }
>>>
>>> /* Used by existing code  */
>>> tree
>>> vect_get_vec_def_for_operand (tree op, gimple *stmt, tree vectype)
>>> {
>>>   vect_is_simple_use(op, loop_vinfo, &def_stmt, &dt); ..and the dump
>>>code
>>>
>>>
>>>   If dt == internal_def || vect_external_def:
>>>       .. Check for BOOLEAN_TYPE ..
>>>       return vect_init_vector (stmt, op, vector_type, NULL);
>>>
>>>   else
>>>     vect_get_vec_def_for_operand_1 (def_stmt, dt)
>>> }
>>>
>>>
>>> Does that look better?
>>
>>Yes!
>>
>>Thanks,
>>Richard.
>>
>
>
> This version of the patch addresses the simple+invariant issues
> (and patch [3/3] optimizes it).
>
> Also includes a small change to handle when the vect pattern has introduced
> new pattern match statements (in vectorizable_live_operation if
> STMT_VINFO_RELATED_STMT is not null then use it instead of stmt).

This is ok.

Thanks,
Richard.

> gcc/
>         * tree-vect-loop.c (vect_analyze_loop_operations): Allow live stmts.
>         (vectorizable_reduction): Check for new relevant state.
>         (vectorizable_live_operation): vectorize live stmts using
>         BIT_FIELD_REF.  Remove special case for gimple assigns stmts.
>         * tree-vect-stmts.c (is_simple_and_all_uses_invariant): New function.
>         (vect_stmt_relevant_p): Check for stmts which are only used live.
>         (process_use): Use of a stmt does not inherit it's live value.
>         (vect_mark_stmts_to_be_vectorized): Simplify relevance inheritance.
>         (vect_analyze_stmt): Check for new relevant state.
>         *tree-vectorizer.h (vect_relevant): New entry for a stmt which is used
>         outside the loop, but not inside it.
>
>         testsuite/
>         * gcc.dg/tree-ssa/pr64183.c: Ensure test does not vectorize.
>         * testsuite/gcc.dg/vect/no-scevccp-vect-iv-2.c: Remove xfail.
>         * gcc.dg/vect/vect-live-1.c: New test.
>         * gcc.dg/vect/vect-live-2.c: New test.
>         * gcc.dg/vect/vect-live-3.c: New test.
>         * gcc.dg/vect/vect-live-4.c: New test.
>         * gcc.dg/vect/vect-live-5.c: New test.
>         * gcc.dg/vect/vect-live-slp-1.c: New test.
>         * gcc.dg/vect/vect-live-slp-2.c: New test.
>         * gcc.dg/vect/vect-live-slp-3.c: New test.
>
>
> Alan.
>