[NewGVN] Don't derive incorrect implications.

[davide]

Summary

The problem is here (debug output):

Processing instruction   %tmp4 = icmp sgt i32 %tmp.0, 0
Created new congruence class for   %tmp4 = icmp sgt i32 %tmp.0, 0 using expression { ExpressionTypeConstant, opcode = 18,  constant = i1 true} at 4 and leader i1 true
New class 4 for expression { ExpressionTypeConstant, opcode = 18,  constant = i1 true}

we believe %tmp1 implies %tmp4

where:

br i1 %tmp1, label %bb2, label %bb7
br i1 %tmp4, label %bb5, label %bb7

because while looking at PredicateInfo stuffs we end up calling isImpliedTrueByMatchingCmp() with the operands swapped,
y less then x implies y less then or equal x but the other way around is not true.

The function has a very confusing name IMHO. I expect isImpliedTrueByMatchingCmp means that *the first argument* is implied by the second and not viceversa. I'm willing to change it, but I wonder if we have other latent bugs in tree because of this.

llvm/trunk/lib/Transforms/Scalar/NewGVN.cpp

if (const auto *PBranch = dyn_cast_or_null<PredicateBranch>(PI)) {
       if (shouldSwapOperands(BranchOp0, BranchOp1)) {
         std::swap(BranchOp0, BranchOp1);
         BranchPredicate = BranchCond->getSwappedPredicate();
       }
       if (BranchOp0 == Op0 && BranchOp1 == Op1) {
         if (PBranch->TrueEdge) {
           // If we know the previous predicate is true and we are in the true
           // edge then we may be implied true or false.
-          if (CmpInst::isImpliedTrueByMatchingCmp(OurPredicate,
-                                                  BranchPredicate)) {
+          if (CmpInst::isImpliedTrueByMatchingCmp(BranchPredicate,
+                                                  OurPredicate)) {
             addPredicateUsers(PI, I);
             return createConstantExpression(
                 ConstantInt::getTrue(CI->getType()));
           }
 
-          if (CmpInst::isImpliedFalseByMatchingCmp(OurPredicate,
-                                                   BranchPredicate)) {
+          if (CmpInst::isImpliedFalseByMatchingCmp(BranchPredicate,
+                                                   OurPredicate)) {
             addPredicateUsers(PI, I);
             return createConstantExpression(
                 ConstantInt::getFalse(CI->getType()));
           }
 
         } else {
           // Just handle the ne and eq cases, where if we have the same
           // operands, we may know something.

llvm/trunk/test/Transforms/NewGVN/pr32852.ll

+; Make sure GVN doesn't incorrectly think the branch terminating
+; bb2 has a constant condition.
+; RUN: opt -S -newgvn %s | FileCheck %s
+
+@a = common global i32 0
+@patatino = private unnamed_addr constant [3 x i8] c"0\0A\00"
+
+define void @tinkywinky() {
+bb:
+  %tmp = load i32, i32* @a
+  %tmp1 = icmp sge i32 %tmp, 0
+  br i1 %tmp1, label %bb2, label %bb7
+bb2:
+  %tmp4 = icmp sgt i32 %tmp, 0
+; CHECK: br i1 %tmp4, label %bb5, label %bb7
+  br i1 %tmp4, label %bb5, label %bb7
+bb5:
+  %tmp6 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @patatino, i32 0, i32 0))
+  br label %bb7
+bb7:
+  ret void
+}
+
+declare i32 @printf(i8*, ...)