<div dir="ltr">Hi,<br><br>Thanks both. David - I think the PatternMatch machinery can certainly be rewritten to use this new helper - I'll do this in a followup commit. Thanks for pointing it out!<div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote">On Mon, 27 Apr 2015 at 17:31 Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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.<br>
<br>
<br>
REPOSITORY<br>
  rL LLVM<br>
<br>
================<br>
Comment at: include/llvm/Analysis/ValueTracking.h:232<br>
@@ +231,3 @@<br>
+<br>
+  /// \brief Specific patterns of select instructions we can match.<br>
+  enum SelectPatternFlavor {<br>
----------------<br>
There's no reason this has to be restricted to select.  Given that, I'd remove it from the comment & naming.<br>
<br>
p.s. w.r.t the term "pattern":  Personally, I might have used "idiom", but I'm not sure the difference matters.<br>
<br>
(edit - given you're just moving the naming from instcombine, please ignore the previous and do not change.)<br>
<br>
================<br>
Comment at: lib/Analysis/ValueTracking.cpp:3047<br>
@@ +3046,3 @@<br>
+    switch (Pred) {<br>
+    default: return SPF_UNKNOWN; // Equality.<br>
+    case ICmpInst::ICMP_UGT:<br>
----------------<br>
Stale comment?<br>
<br>
================<br>
Comment at: lib/Analysis/ValueTracking.cpp:3059<br>
@@ +3058,3 @@<br>
+<br>
+  // (icmp X, Y) ? Y : X<br>
+  if (TrueVal == CmpRHS && FalseVal == CmpLHS) {<br>
----------------<br>
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.<br>
<br>
<a href="http://reviews.llvm.org/D9294" target="_blank">http://reviews.llvm.org/D9294</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>