On Wed, Apr 11, 2012 at 11:28 AM, Bin.Cheng <amker.ch...@gmail.com> wrote:
> On Mon, Apr 9, 2012 at 7:02 PM, Richard Guenther
> <richard.guent...@gmail.com> wrote:
>> On Mon, Apr 9, 2012 at 8:00 AM, Bin.Cheng <amker.ch...@gmail.com> wrote:
>>> On Fri, Mar 30, 2012 at 5:43 PM, Bin.Cheng <amker.ch...@gmail.com> wrote:
>
>>>
>>> Hi Richard,
>>> I am testing a patch to sink load of memory to proper basic block.
>>> Everything goes fine except auto-vectorization, sinking of load sometime
>>> corrupts the canonical form of data references. I haven't touched auto-vec
>>> before and cannot tell whether it's good or bad to do sink before auto-vec.
>>> For example, the slp-cond-1.c
>>>
>>> <bb 3>:
>>> # i_39 = PHI <i_32(11), 0(2)>
>>> D.5150_5 = i_39 * 2;
>>> D.5151_10 = D.5150_5 + 1;
>>> D.5153_17 = a[D.5150_5];
>>> D.5154_19 = b[D.5150_5];
>>> if (D.5153_17 >= D.5154_19)
>>> goto <bb 9>;
>>> else
>>> goto <bb 4>;
>>>
>>> <bb 9>:
>>> d0_6 = d[D.5150_5]; <-----this is sunk from bb3
>>> goto <bb 5>;
>>>
>>> <bb 4>:
>>> e0_8 = e[D.5150_5]; <-----this is sunk from bb3
>>>
>>> <bb 5>:
>>> # d0_2 = PHI <d0_6(9), e0_8(4)>
>>> k[D.5150_5] = d0_2;
>>> D.5159_26 = a[D.5151_10];
>>> D.5160_29 = b[D.5151_10];
>>> if (D.5159_26 >= D.5160_29)
>>> goto <bb 10>;
>>> else
>>> goto <bb 6>;
>>>
>>>
>>> <bb 10>:
>>> d1_11 = d[D.5151_10]; <-----this is sunk from bb3
>>> goto <bb 7>;
>>>
>>> <bb 6>:
>>> e1_14 = e[D.5151_10]; <-----this is sunk from bb3
>>>
>>> <bb 7>:
>>> .......
>>>
>>> I will look into auto-vect but not sure how to handle this case.
>>>
>>> Any comments? Thanks very much.
>>
>> Simple - the vectorizer expects empty latch blocks. So simply
>> never sink stuff into latch-blocks - I think the current code already
>> tries to avoid that for regular computations.
>
> Seems not the story. The sinkto basic block is not latch basic block at all.
>
> The problem is about if-conversion, code of the case in slp-cond-1.c is like:
>
> __attribute__((noinline, noclone)) void
> f4 (void)
> {
> int i;
> for (i = 0; i < N/2; ++i)
> {
> int d0 = d[2*i], e0 = e[2*i];
> int d1 = d[2*i+1], e1 = e[2*i+1];
> k[2*i] = a[2*i] >= b[2*i] ? d0 : e0; <------example
> k[2*i+1] = a[2*i+1] >= b[2*i+1] ? d1 : e1;
> }
> }
> It is strictly formed in conditional operations, just like the output
> of if-conversion pass before auto-vec.
>
> Now sink pass sinks load of d0/e0 into then/else branch, since they
> are partial dead code in the opposite branch, which corrupts the
> canonical form.
>
> Ideally, if-conversion pass should handle the sinked code and collapse
> if-then-else into conditional operations.
> But as showed by dump of if-conversion pass before auto-vect:
>
> <bb 3>:
> # i_39 = PHI <i_32(10), 0(2)>
> # ivtmp.137_41 = PHI <ivtmp.137_38(10), 16(2)>
> D.5150_5 = i_39 * 2;
> D.5151_10 = D.5150_5 + 1;
> D.5153_17 = a[D.5150_5];
> D.5154_19 = b[D.5150_5];
> if (D.5153_17 >= D.5154_19)
> goto <bb 4>;
> else
> goto <bb 5>;
>
> <bb 4>:
> d0_6 = d[D.5150_5];
> goto <bb 6>;
>
> <bb 5>:
> e0_8 = e[D.5150_5];
>
> <bb 6>:
> # d0_2 = PHI <d0_6(4), e0_8(5)>
> k[D.5150_5] = d0_2;
>
> It does not work as expected.
>
> So I think sink pass is fine for this case, it is the if-conversion
> pass need further investigation.
> Comments? Thanks.
Turns out if-conversion checks whether gimple statement traps or not.
For the statement "d0_6 = d[D.5150_5];", it assumes rhs might trap,
because it sees the index not INTEGER_CST.
Two questions:
1, The check can be enhanced in gimple_could_trap_p_1 for ARRAY_REF.
2, Should I check this before sinking load from memory? If yes, then why sink of
store does not do such check?
Again, please correct if I missed something important.
Thanks very much.
--
Best Regards.
