[PATCH] D29682: NewGVN: Start making use of predicateinfo pass.

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Scalar/NewGVN.cpp:882
+    } else if ((PBranch->TrueEdge && CmpInst::isTrueWhenEqual(Predicate)) ||
+               (!PBranch->TrueEdge && CmpInst::isFalseWhenEqual(Predicate))) {
+      // If we are *not* a copy of the comparison, we may equal to the other
----------------
For the record: The above part is true, but this is wrong.
It's only true for equality (ICMP_EQ and ICMP_NE)
After your comment below, I wrote a little fuzzer that tested every variant, and .... it flagged this (the part you mentioned below is definitely correct)
Trivial example:
 %tmp3 = load i32, i32* %tmp, align 4
 %tmp5 = icmp ult i32 %tmp3, 16

%tmp5 is false when equal.
That does not imply that tmp3 == 16.

The more i stare at this, the more unhappy i am that all this logic is not somewhere else.
I added it to my todo list to see about trying to plumb this through various parts of simplifyinst, which already has well tested logic for these simplifications, it just needs to know we know something about the value.



https://reviews.llvm.org/D29682