[llvm] 97a7bc5 - [gvn] Precisely propagate equalities to phi operands

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2021-03-08T08:59:00-08:00
New Revision: 97a7bc5831152eabcd61e5b970c054ce67c2c91b

URL: https://github.com/llvm/llvm-project/commit/97a7bc5831152eabcd61e5b970c054ce67c2c91b
DIFF: https://github.com/llvm/llvm-project/commit/97a7bc5831152eabcd61e5b970c054ce67c2c91b.diff

LOG: [gvn] Precisely propagate equalities to phi operands

The code used for propagating equalities (e.g. assume facts) was conservative in two ways - one of which this patch fixes. Specifically, it shifts the code reasoning about whether a use is dominated by the end of the assume block to consider phi uses to exist on the predecessor edge. This matches the dominator tree handling for dominates(Edge, Use), and simply extends it to dominates(BB, Use).

Note that the decision to use the end of the block is itself a conservative choice. The more precise option would be to use the later of the assume and the value, and replace all uses after that. GVN handles that case separately (with the replace operand mechanism) because it used to be expensive to ask dominator questions within blocks. With the new instruction ordering support, we should probably rewrite this code at some point to simplify.

Differential Revision: https://reviews.llvm.org/D98082

Added: 
    

Modified: 
    llvm/include/llvm/IR/Dominators.h
    llvm/lib/IR/Dominators.cpp
    llvm/lib/Transforms/Utils/Local.cpp
    llvm/test/Transforms/GVN/phi.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Dominators.h b/llvm/include/llvm/IR/Dominators.h
index 08dbccaf2c01..4d140c3ad0f2 100644
--- a/llvm/include/llvm/IR/Dominators.h
+++ b/llvm/include/llvm/IR/Dominators.h
@@ -165,6 +165,9 @@ class DominatorTree : public DominatorTreeBase<BasicBlock, false> {
   // Ensure base-class overloads are visible.
   using Base::dominates;
 
+  /// Return true if the (end of the) basic block BB dominates the use U.
+  bool dominates(const BasicBlock *BB, const Use &U) const;
+
   /// Return true if value Def dominates use U, in the sense that Def is
   /// available at U, and could be substituted as the used value without
   /// violating the SSA dominance requirement.

diff  --git a/llvm/lib/IR/Dominators.cpp b/llvm/lib/IR/Dominators.cpp
index 5d17d6ced3c5..ace708b252c7 100644
--- a/llvm/lib/IR/Dominators.cpp
+++ b/llvm/lib/IR/Dominators.cpp
@@ -112,6 +112,16 @@ bool DominatorTree::invalidate(Function &F, const PreservedAnalyses &PA,
            PAC.preservedSet<CFGAnalyses>());
 }
 
+bool DominatorTree::dominates(const BasicBlock *BB, const Use &U) const {
+  Instruction *UserInst = cast<Instruction>(U.getUser());
+  if (auto *PN = dyn_cast<PHINode>(UserInst))
+    // A phi use using a value from a block is dominated by the end of that
+    // block.  Note that the phi's parent block may not be.
+    return dominates(BB, PN->getIncomingBlock(U));
+  else
+    return properlyDominates(BB, UserInst->getParent());
+}
+
 // dominates - Return true if Def dominates a use in User. This performs
 // the special checks necessary if Def and User are in the same basic block.
 // Note that Def doesn't dominate a use in Def itself!

diff  --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 93e5b941c208..446149f42ce3 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -2683,11 +2683,10 @@ unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To,
 unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To,
                                         DominatorTree &DT,
                                         const BasicBlock *BB) {
-  auto ProperlyDominates = [&DT](const BasicBlock *BB, const Use &U) {
-    auto *I = cast<Instruction>(U.getUser())->getParent();
-    return DT.properlyDominates(BB, I);
+  auto Dominates = [&DT](const BasicBlock *BB, const Use &U) {
+    return DT.dominates(BB, U);
   };
-  return ::replaceDominatedUsesWith(From, To, BB, ProperlyDominates);
+  return ::replaceDominatedUsesWith(From, To, BB, Dominates);
 }
 
 bool llvm::callsGCLeafFunction(const CallBase *Call,

diff  --git a/llvm/test/Transforms/GVN/phi.ll b/llvm/test/Transforms/GVN/phi.ll
index b978281effbe..6c2439968fec 100644
--- a/llvm/test/Transforms/GVN/phi.ll
+++ b/llvm/test/Transforms/GVN/phi.ll
@@ -37,10 +37,8 @@ define i64 @test2(i1 %c, i64 %a, i64 %b) {
 ; CHECK:       untaken:
 ; CHECK-NEXT:    br label [[MERGE]]
 ; CHECK:       merge:
-; CHECK-NEXT:    [[PHI1:%.*]] = phi i64 [ [[A]], [[TAKEN]] ], [ [[B:%.*]], [[UNTAKEN]] ]
-; CHECK-NEXT:    [[PHI2:%.*]] = phi i64 [ 0, [[TAKEN]] ], [ [[B]], [[UNTAKEN]] ]
-; CHECK-NEXT:    [[RET:%.*]] = sub i64 [[PHI1]], [[PHI2]]
-; CHECK-NEXT:    ret i64 [[RET]]
+; CHECK-NEXT:    [[PHI1:%.*]] = phi i64 [ 0, [[TAKEN]] ], [ [[B:%.*]], [[UNTAKEN]] ]
+; CHECK-NEXT:    ret i64 0
 ;
   br i1 %c, label %taken, label %untaken
 taken:
@@ -134,8 +132,7 @@ define i64 @test5(i1 %c, i64 %a) {
 ; CHECK:       untaken:
 ; CHECK-NEXT:    br label [[MERGE]]
 ; CHECK:       merge:
-; CHECK-NEXT:    [[PHI:%.*]] = phi i64 [ [[A]], [[TAKEN]] ], [ 0, [[UNTAKEN]] ]
-; CHECK-NEXT:    ret i64 [[PHI]]
+; CHECK-NEXT:    ret i64 0
 ;
   br i1 %c, label %taken, label %untaken
 taken:
@@ -159,8 +156,7 @@ define i64 @test6(i1 %c, i64 %a) {
 ; CHECK:       untaken:
 ; CHECK-NEXT:    br label [[MERGE]]
 ; CHECK:       merge:
-; CHECK-NEXT:    [[PHI:%.*]] = phi i64 [ [[A]], [[TAKEN]] ], [ 0, [[UNTAKEN]] ]
-; CHECK-NEXT:    ret i64 [[PHI]]
+; CHECK-NEXT:    ret i64 0
 ;
   br i1 %c, label %taken, label %untaken
 taken: