Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes:
> On Fri, 23 Aug 2019 at 18:15, Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> writes:
>> > On Thu, 22 Aug 2019 at 16:44, Richard Biener <richard.guent...@gmail.com>
>> > wrote:
>> >> It looks a bit odd to me. I'd have expected it to work by generating
>> >> the stmts as before in the vectorizer and then on the stmts we care
>> >> invoke vn_visit_stmt that does both value-numbering and elimination.
>> >> Alternatively you could ask the VN state to generate the stmt for
>> >> you via vn_nary_build_or_lookup () (certainly that needs a bit more
>> >> work). One complication might be availability if you don't value-number
>> >> all stmts in the block, but well. I'm not sure constraining to a single
>> >> block is necessary - I've thought of having a "CSE"ing gimple_build
>> >> for some time (add & CSE new stmts onto a sequence), so one
>> >> should keep this mode in mind when designing the one working on
>> >> an existing BB. Note as you write it it depends on visiting the
>> >> stmts in proper order - is that guaranteed when for example
>> >> vectorizing SLP?
>> > Hi,
>> > Indeed, I wrote the function with assumption that, stmts would be
>> > visited in proper order.
>> > This doesn't affect SLP currently, because call to vn_visit_stmt in
>> > vect_transform_stmt is
>> > conditional on cond_to_vec_mask, which is only allocated inside
>> > vect_transform_loop.
>> > But I agree we could make it more general.
>> > AFAIU, the idea of constraining VN to single block was to avoid using defs
>> > from
>> > non-dominating scalar stmts during outer-loop vectorization.
>>
>> Maybe we could do the numbering in a separate walk immediately before
>> the transform phase instead.
> Um sorry, I didn't understand. Do you mean we should do dom based VN
> just before transform phase
> or run full VN ?
No, I just meant that we could do a separate walk of the contents
of the basic block:
> @@ -8608,6 +8609,8 @@ vect_transform_loop (loop_vec_info loop_vinfo)
> {
> basic_block bb = bbs[i];
> stmt_vec_info stmt_info;
> + vn_bb_init (bb);
> + loop_vinfo->cond_to_vec_mask = new cond_vmask_map_type (8);
>
...here, rather than doing it on the fly during vect_transform_stmt
itself. The walk should be gated on LOOP_VINFO_FULLY_MASKED_P so that
others don't have to pay the compile-time penalty. (Same for
cond_to_vec_mask itself really.)
Thanks,
Richard
