Re: [Patch] Avoid is_simple_use bug in vectorizable_live_operation

[Alan Hayward <alan dot hayward at arm dot com>]

On 26/10/2015 13:35, "Richard Biener" <richard.guenther@gmail.com> wrote:

>On Mon, Oct 26, 2015 at 1:33 PM, Alan Hayward <alan.hayward@arm.com>
>wrote:
>> There is a potential bug in vectorizable_live_operation.
>>
>> Consider the case where the first op for stmt is valid, but the second
>>is
>> null.
>> The first time through the for () loop, it will call out to
>> vect_is_simple_use () which will set dt.
>> The second time, because op is null, vect_is_simple_use () will not be
>> called.
>> However, dt is still set to a valid value, therefore the loop will
>> continue.
>>
>> Note this is different from the case where the first op is null, which
>> will cause the loop not call vect_is_simple_use () and then return
>>false.
>>
>> It is possible that this was intentional, however that is not clear from
>> the code.
>>
>> The fix is to simply ensure dt is initialized to a default value on each
>> iteration.
>
>I think the patch is a strict improvement, thus OK.  Still a NULL operand
>is not possible in GIMPLE so the op && check is not necessary.  The way
>it iterates over all stmt uses is a bit scary anyway.  As it is ok with
>all invariants it should probably simply do sth like
>
>   FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_USE)
>     if (!vect_is_simple_use (op, ....))
>
>and be done with that.  Unvisited uses can only be constants (ok).
>
>Care to rework the funtion like that if you are here?
>

Ok, Iâve updated as requested.


Cheers,
Alan.

Attachment: avoid_issimple2.patch
Description: Binary data