[PATCH] D27846: [SLP] Support for horizontal min/max reduction

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In https://reviews.llvm.org/D27846#661215, @mkuper wrote:

> This is another example of what I was talking about re smaller patches.
>
> You could do things like name changes (Reduction -> ArithmeticReduction) or moving comments around (from inside the body of getReductionCost to above the declaration) in separate NFC patches. In a lot of cases they're simple enough not to require pre-commit review, and it makes reviewing actual meaningful patches much much easier, because the diff only contains relevant things. You could also start with a patch that only supports one kind of min/max reduction, and add the rest of them as a follow-up.
>
> I'm sorry if it seems like I'm being petty. In fact I'm really interested in getting all of this stuff in. But the SLP vectorizer is not the simplest or the clearest piece of code to begin with, and I'm not deeply familiar with its nuances, so it's really hard for me to meaningfully review SLP patches if they also contain noise, and if they do more than one thing. If there's someone else who's capable of reviewing and LGTMing these patches as is, I don't oppose it. But I can't.


Michael, no problems at all. Will do my best to split this and other patches into several smaller pieces, but I can't guarantee that it will be possible to do in all cases.


https://reviews.llvm.org/D27846