Have GVN also do condition propagation when the right-hand side is not

a constant.  This fixes PR1768.


git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@151713 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 77 insertions, 11 deletions

diff --git a/lib/Transforms/Scalar/GVN.cpp b/lib/Transforms/Scalar/GVN.cpp
index 702d1d700e..37e970b8b6 100644
--- a/lib/Transforms/Scalar/GVN.cpp
+++ b/lib/Transforms/Scalar/GVN.cpp
@@ -1972,21 +1972,30 @@ bool GVN::propagateEquality(Value *LHS, Value *RHS, BasicBlock *Root) {
   if (isa<Constant>(LHS) && isa<Constant>(RHS))
     return false;
 
-  // Make sure that any constants are on the right-hand side.  In general the
-  // best results are obtained by placing the longest lived value on the RHS.
-  if (isa<Constant>(LHS))
+  // Prefer a constant on the right-hand side, or an Argument if no constants.
+  if (isa<Constant>(LHS) || (isa<Argument>(LHS) && !isa<Constant>(RHS)))
     std::swap(LHS, RHS);
-
-  // If neither term is constant then bail out.  This is not for correctness,
-  // it's just that the non-constant case is much less useful: it occurs just
-  // as often as the constant case but handling it hardly ever results in an
-  // improvement.
-  if (!isa<Constant>(RHS))
-    return false;
+  assert((isa<Argument>(LHS) || isa<Instruction>(LHS)) && "Unexpected value!");
+
+  // If there is no obvious reason to prefer the left-hand side over the right-
+  // hand side, ensure the longest lived term is on the right-hand side, so the
+  // shortest lived term will be replaced by the longest lived.  This tends to
+  // expose more simplifications.
+  uint32_t LVN = VN.lookup_or_add(LHS);
+  if ((isa<Argument>(LHS) && isa<Argument>(RHS)) ||
+      (isa<Instruction>(LHS) && isa<Instruction>(RHS))) {
+    // Move the 'oldest' value to the right-hand side, using the value number as
+    // a proxy for age.
+    uint32_t RVN = VN.lookup_or_add(RHS);
+    if (LVN < RVN) {
+      std::swap(LHS, RHS);
+      LVN = RVN;
+    }
+  }
 
   // If value numbering later deduces that an instruction in the scope is equal
   // to 'LHS' then ensure it will be turned into 'RHS'.
-  addToLeaderTable(VN.lookup_or_add(LHS), RHS, Root);
+  addToLeaderTable(LVN, RHS, Root);
 
   // Replace all occurrences of 'LHS' with 'RHS' everywhere in the scope.  As
   // LHS always has at least one use that is not dominated by Root, this will
diff --git a/test/Transforms/GVN/condprop.ll b/test/Transforms/GVN/condprop.ll
index 97a0d31e5e..b22675b47c 100644
--- a/test/Transforms/GVN/condprop.ll
+++ b/test/Transforms/GVN/condprop.ll
@@ -175,3 +175,60 @@ different:
 ; CHECK: ret i1 false
   ret i1 %cmp3
 }
+
+; PR1768
+; CHECK: @test9
+define i32 @test9(i32 %i, i32 %j) {
+  %cmp = icmp eq i32 %i, %j
+  br i1 %cmp, label %cond_true, label %ret
+
+cond_true:
+  %diff = sub i32 %i, %j
+  ret i32 %diff
+; CHECK: ret i32 0
+
+ret:
+  ret i32 5
+; CHECK: ret i32 5
+}
+
+; PR1768
+; CHECK: @test10
+define i32 @test10(i32 %j, i32 %i) {
+  %cmp = icmp eq i32 %i, %j
+  br i1 %cmp, label %cond_true, label %ret
+
+cond_true:
+  %diff = sub i32 %i, %j
+  ret i32 %diff
+; CHECK: ret i32 0
+
+ret:
+  ret i32 5
+; CHECK: ret i32 5
+}
+
+declare i32 @yogibar()
+
+; CHECK: @test11
+define i32 @test11(i32 %x) {
+  %v0 = call i32 @yogibar()
+  %v1 = call i32 @yogibar()
+  %cmp = icmp eq i32 %v0, %v1
+  br i1 %cmp, label %cond_true, label %next
+
+cond_true:
+  ret i32 %v1
+; CHECK: ret i32 %v0
+
+next:
+  %cmp2 = icmp eq i32 %x, %v0
+  br i1 %cmp2, label %cond_true2, label %next2
+
+cond_true2:
+  ret i32 %v0
+; CHECK: ret i32 %x
+
+next2:
+  ret i32 0
+}