This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Missed optimization in PRE?


On Wed, Apr 11, 2012 at 10:05 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Wed, Apr 11, 2012 at 11:28 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>> On Mon, Apr 9, 2012 at 7:02 PM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Mon, Apr 9, 2012 at 8:00 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
>>>> On Fri, Mar 30, 2012 at 5:43 PM, Bin.Cheng <amker.cheng@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.

Yes.  After sinking the load is no longer executed unconditionally but
if-conversion would make it so.  This is information loss caused by
sinking.

> Two questions:
> 1, The check can be enhanced in gimple_could_trap_p_1 for ARRAY_REF.

No, the index may be out-of-bounds.

> 2, Should I check this before sinking load from memory? If yes, then why sink of
> store does not do such check?

Sinking is never a problem - the code will only be executed less times.  The
issue with if-conversion is that it speculates the loads / stores, so they may
not trap if they were originally not executed.

So this is a pass ordering issue - sinking and if-conversion have different
conflicting goals.  Btw, you also make RTL if-conversion harder.  I suppose
you should try to avoid messing up if-conversion possibilities so early,
thus, not sink in these cases.  The same issue is present
for non-loads that are possibly trapping.  So I'm not even sure we can easily
detect these cases - apart from never sinking possibly trapping stmts.

At least you could say that the side-effect of trapping has to be preserved
(note that we do not generally do that, which you might consider a bug).

Richard.

> Again, please correct if I missed something important.
> Thanks very much.
>
> --
> Best Regards.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]