[PATCH] D22341: MinMax Index Pattern Identification

[Mohammed Agabaria via llvm-commits]

magabari added a comment.

In https://reviews.llvm.org/D22341#495519, @mkuper wrote:

> Two notes, before I start the actual review:
>
> First - next time, please don't add llvm-commits after the patch has already been put up for review.
>  Rather, cancel the existing review, and start a new one with llvm-commits as a subscriber to begin with. Some people feel strongly about having the patch appear in the review thread on llvm-commits.
>
> Second - is this a complete patch? The "return Maybe; //TODO: FIX" looks fishy. And I didn't see anything actually using the Maybe return value.
>  If this isn't complete, then I'm not entirely sure what's up for review. In that case, please start a new review with the complete patch - with llvm-commits subscribed from the beginning, per point 1. If it is supposed to be complete, then can you explain what the deal with the Maybe is?
>  (Breaking a patch into smaller units is generally good - but each such unit should be standalone, and this patch doesn't look like it.)


Regarding your first point Okay.
Second point: This is part 1 from 2 this patch deals with the identification of Min Max Index pattern only (it won't vectorize it) although it's a standalone commit but i think i will wait and commit both parts together.
The "Maybe" is just a bad name, need to understand that MinMax Index Pattern rely on the fact that we have found MinMax pattern, so in the first phase i can only say that its a good candidate for MinMax Index reduction (this is why i return Maybe i think it's better to return Candidate) and i put this in ExtraVerificationRequiredList to verify that we have found Min Max pattern. In the 2nd phase i move over all those candidates and make sure that they are real reductions.


Repository:
  rL LLVM

https://reviews.llvm.org/D22341