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

James Molloy james at jamesmolloy.co.uk
Mon Apr 27 09:40:14 PDT 2015


Hi,

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!

Cheers,

James

On Mon, 27 Apr 2015 at 17:31 Philip Reames <listmail at philipreames.com>
wrote:

> 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/
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150427/18cd0c76/attachment.html>


More information about the llvm-commits mailing list