[PATCH] D17873: [InstCombine] (icmp sgt smin(PosA, B) 0) -> (icmp sgt B 0)

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:3181
@@ +3180,3 @@
+        if (SPR.Flavor == SPF_SMIN) {
+          if (isKnownNonNegative(A, DL) && isKnownNonZero(A, DL))
+            return new ICmpInst(I.getPredicate(), B, CI);
----------------
sanjoy wrote:
> This is more work for you, but I think the right thing to do here is introduce an `isKnownPositive` helper in ValueTracking.h -- doing two recursive walks on the same expression seems wasteful.
> 
> I'd say lets start with a simple `isKnownPositive` implementation (maybe one that just looks at `!range` metadata and `computeKnownBits`), which we can evolve in the future.
> 
I disagree that introducing yet another independent implementation is a good idea.  

To be blunt, the extra work to make the approach you're requesting work isn't worth it for this patch.  I'll just abandon the patch instead because it turns out not to address the case I actually care about.


http://reviews.llvm.org/D17873