[PATCH] Rip min/max pattern matching out of InstCombine and into ValueTracking.

Philip Reames listmail at philipreames.com
Mon Apr 27 09:25:49 PDT 2015


LGTM.  If you want to reframe things w/PatternMatch, feel free, but please do that in a second commit.  Having the move be clearly NFC is worthwhile.


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/Analysis/ValueTracking.h:232
@@ +231,3 @@
+  
+  /// \brief Specific patterns of select instructions we can match.
+  enum SelectPatternFlavor {
----------------
There's no reason this has to be restricted to select.  Given that, I'd remove it from the comment & naming.  

p.s. w.r.t the term "pattern":  Personally, I might have used "idiom", but I'm not sure the difference matters.  

(edit - given you're just moving the naming from instcombine, please ignore the previous and do not change.)

================
Comment at: lib/Analysis/ValueTracking.cpp:3047
@@ +3046,3 @@
+    switch (Pred) {
+    default: return SPF_UNKNOWN; // Equality.
+    case ICmpInst::ICMP_UGT:
----------------
Stale comment?

================
Comment at: lib/Analysis/ValueTracking.cpp:3059
@@ +3058,3 @@
+
+  // (icmp X, Y) ? Y : X
+  if (TrueVal == CmpRHS && FalseVal == CmpLHS) {
----------------
You could likely combine this with the above by canonicalizing the predicate and operand direction.  I'm fine with the code as is, just pointing that out.

http://reviews.llvm.org/D9294

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list