[PATCH] D22341: MinMax Index Pattern Identification

Michael Kuperstein via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 16:37:06 PDT 2016


mkuper added a comment.

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.)


Repository:
  rL LLVM

https://reviews.llvm.org/D22341





More information about the llvm-commits mailing list