Re: [PATCH] Re: Vectorizer question: DIV to RSHIFT conversion (take 2)

[Ira Rosen <IRAR at il dot ibm dot com>]

Jakub Jelinek <jakub@redhat.com> wrote on 15/12/2011 12:54:29 PM:

> Perhaps it would be even cleaner to get rid of the pattern stmt and def
stmt
> seq distinction and just have pattern as whole be represented as
gimple_seq,
> but perhaps that cleanup can be deferred for later.

Sounds good.

> This patch also fixes
> a problem where vect_determine_vectorization_factor would iterate the
same
> stmt twice - for some reason both the original stmt and pattern stmt (and
> def stmt) are marked as relevant,

Do you have a testcase where the original stmt is marked as relevant? It
shouldn't be that way.

> and we iterate on the same original stmt
> not just 3 times, but 4 times - first iteration on the original stmt,
> setting analyze_pattern_stmt, second iteration starts with the pattern
stmt
> but clears analyze_pattern_stmt, then sees it has a def stmt and thus
> runs on the def stmt, third iteration with pattern_def set on entry
> again on the original stmt, setting analyze_pattern_stmt again and last
one
> on the pattern stmt (because pattern_def was already set on entry and it
> clears it).

In case the original stmt is not marked as relevant we don't really
analyze/transform the same stmt twice (and certainly not 4 times).

> Sounds like those two routines
> (vect_determine_vectorization_factor and vect_transform_loop) would be
> bettern rewritten with a helper that would handle a single stmt/stmt_info
> pair and we would just call that helper on all the original/pattern/def
> stmts we want to handle.

I agree.

>
> 2011-12-15  Jakub Jelinek  <jakub@redhat.com>
>
>    * tree-vectorizer.h (struct _stmt_vec_info): Remove pattern_def_stmt
>    field, add pattern_def_seq.
>    (STMT_VINFO_PATTERN_DEF_STMT): Remove.
>    (STMT_VINFO_PATTERN_DEF_SEQ): Define.
>    (NUM_PATTERNS): Bump to 10.
>    * tree-vect-loop.c (vect_determine_vectorization_factor,
>    vect_transform_loop): Adjust for pattern def changing from a single
>    gimple stmt to gimple_seq.
>    * tree-vect-stmts.c (vect_analyze_stmt, new_stmt_vec_info,
>    free_stmt_vec_info): Likewise.
>    * tree-vect-patterns.c (vect_recog_over_widening_pattern,
>    vect_recog_vector_vector_shift_pattern,
>    vect_recog_mixed_size_cond_pattern, adjust_bool_pattern_cast,
>    adjust_bool_pattern, vect_mark_pattern_stmts): Likewise.
>    (vect_recog_sdivmod_pow2_pattern): New function.
>    (vect_vect_recog_func_ptrs): Add it.

The patch looks ok to me, but I wonder if it's appropriate for the current
stage.

Thanks,
Ira