[PATCH] D29449: [SLP] Generalization of vectorization of CmpInst operands, NFC.

[Michael Kuperstein via llvm-commits]

Oh, ok, I see. So starting from a cmp runs into the same problem as
starting from a binaryop would cause for an add reduction.

I'm actually not sure why we're doing the walk top-to-bottom - it sounds
like the wrong direction if we're looking for the "largest tree".
It looks like originally we were only looking at phi's, which meant the
walk was from the start of the basic block to the last phi. So it made
sense when the code was written. But it doesn't really make sense to me
now. (Unless it helps in terms of compile-time, somehow?)

Can we change the direction of the walk?

Thoughts, anyone else? Hal?

On Tue, Feb 7, 2017 at 9:03 PM, Alexey Bataev <a.bataev at hotmail.com> wrote:

> Because we start from top-to-bottom analysis and vectorize the arguments
> of CmpInst. After that we ran into the second CmpInst, but we already can't
> do anything with these CmpInsts, because the first CmpInst is vectorized
> already and we can't recognise the min/max pattern.
>
> Best regards,
> Alexey Bataev
>
> 8 февр. 2017 г., в 0:33, Michael Kuperstein <mkuper at google.com>
> написал(а):
>
> Why does it block that?
>
> On Tue, Feb 7, 2017 at 1:19 PM, Alexey Bataev <a.bataev at hotmail.com>
> wrote:
>
>> The first option is not suitable, it blocks min/max reduction
>> vectorization.
>>
>> Best regards,
>> Alexey Bataev
>>
>> > 8 февр. 2017 г., в 0:09, Michael Kuperstein via Phabricator <
>> reviews at reviews.llvm.org> написал(а):
>> >
>> > mkuper added a comment.
>> >
>> >> What should I do then?
>> >
>> > Short term - maybe nothing?
>> > Is this patch blocking anything? I understand this is part of the work
>> to support min/max reductions, but why is it necessary? Can we go forward
>> with that without regressing any existing cases?
>> >
>> > Longer term - it would probably be good to try to come up with a saner,
>> or at least, more principled way to do root selection, that also doesn't
>> cause us to look at instructions several times. I don't think adding more
>> ad-hoc cases (CallInst) is the way to go. I'm fairly sure we can come up
>> with other examples like this.
>> >
>> >
>> > https://reviews.llvm.org/D29449
>> >
>> >
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170209/2ac8570b/attachment-0001.html>