-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[GVN] Share equality propagation for assume and condition #161639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
b2bb2f1
to
bd6f434
Compare
bd6f434
to
6d00400
Compare
// Cached information for anything that uses NotCmp will be invalid. | ||
if (MD) | ||
MD->invalidateCachedPointerInfo(NotCmp); | ||
for (const auto &Entry : LeaderTable.getLeaders(Num)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To explain what is going on here: I initially changed this to look for leaders in DominatedBlocks instead, but that caused regressions for the case where the edge is non-dominating, but there is a phi use of the inverted condition.
So instead I explicitly loop over the leaders and inspect the ones that could potentially have replaceable uses. Worth noting that what findLeader() does internally is also loop over the leaders and perform a dominance check, we just need more flexibility here for the dominance condition to use.
This change is what accounts for the improvements in llvm-opt-benchmark.
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesGVN currently has two different implementation of equality propagation. One of them is used for branch conditions (dominating an edge), which performs replacements across multiple blocks. This is also used for assumes to handle uses outside the current block. However, uses inside the block are handled using a completely separate implementation, which involves populating a replacement map and then checking it for individual instructions during normal GVN. While this approach generally makes sense, it is kind of pointless if we already do a use walk to handle the cross-block case anyway. This PR generalizes propagateEquality() to accept either a BasicBlockEdge or an Instruction* and replace dominated users. This removes the need for special handling of uses in the same block for assumes, as they're covered by instruction dominance, and ensures that both implementations do not go out of sync. Full diff: https://github.com/llvm/llvm-project/pull/161639.diff 5 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Scalar/GVN.h b/llvm/include/llvm/Transforms/Scalar/GVN.h
index 245414935bc0f..8599499f29d6d 100644
--- a/llvm/include/llvm/Transforms/Scalar/GVN.h
+++ b/llvm/include/llvm/Transforms/Scalar/GVN.h
@@ -28,6 +28,7 @@
#include <cstdint>
#include <optional>
#include <utility>
+#include <variant>
#include <vector>
namespace llvm {
@@ -321,11 +322,6 @@ class GVNPass : public PassInfoMixin<GVNPass> {
};
LeaderMap LeaderTable;
- // Block-local map of equivalent values to their leader, does not
- // propagate to any successors. Entries added mid-block are applied
- // to the remaining instructions in the block.
- SmallMapVector<Value *, Value *, 4> ReplaceOperandsWithMap;
-
// Map the block to reversed postorder traversal number. It is used to
// find back edge easily.
DenseMap<AssertingVH<BasicBlock>, uint32_t> BlockRPONumber;
@@ -400,9 +396,9 @@ class GVNPass : public PassInfoMixin<GVNPass> {
void verifyRemoved(const Instruction *I) const;
bool splitCriticalEdges();
BasicBlock *splitCriticalEdges(BasicBlock *Pred, BasicBlock *Succ);
- bool replaceOperandsForInBlockEquality(Instruction *I) const;
- bool propagateEquality(Value *LHS, Value *RHS, const BasicBlockEdge &Root,
- bool DominatesByEdge);
+ bool
+ propagateEquality(Value *LHS, Value *RHS,
+ const std::variant<BasicBlockEdge, Instruction *> &Root);
bool processFoldableCondBr(BranchInst *BI);
void addDeadBlock(BasicBlock *BB);
void assignValNumForDeadCode();
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index 3f5f4278a2766..74c2a5c62016e 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -452,6 +452,9 @@ LLVM_ABI unsigned replaceDominatedUsesWith(Value *From, Value *To,
LLVM_ABI unsigned replaceDominatedUsesWith(Value *From, Value *To,
DominatorTree &DT,
const BasicBlock *BB);
+LLVM_ABI unsigned replaceDominatedUsesWith(Value *From, Value *To,
+ DominatorTree &DT,
+ const Instruction *I);
/// Replace each use of 'From' with 'To' if that use is dominated by
/// the given edge and the callback ShouldReplace returns true. Returns the
/// number of replacements made.
@@ -465,6 +468,10 @@ LLVM_ABI unsigned replaceDominatedUsesWithIf(
Value *From, Value *To, DominatorTree &DT, const BasicBlock *BB,
function_ref<bool(const Use &U, const Value *To)> ShouldReplace);
+LLVM_ABI unsigned replaceDominatedUsesWithIf(
+ Value *From, Value *To, DominatorTree &DT, const Instruction *I,
+ function_ref<bool(const Use &U, const Value *To)> ShouldReplace);
+
/// Return true if this call calls a gc leaf function.
///
/// A leaf function is a function that does not safepoint the thread during its
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 26e17cc849bff..1e81a92845905 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -2083,13 +2083,6 @@ bool GVNPass::processNonLocalLoad(LoadInst *Load) {
return Changed;
}
-static bool hasUsersIn(Value *V, BasicBlock *BB) {
- return any_of(V->users(), [BB](User *U) {
- auto *I = dyn_cast<Instruction>(U);
- return I && I->getParent() == BB;
- });
-}
-
bool GVNPass::processAssumeIntrinsic(AssumeInst *IntrinsicI) {
Value *V = IntrinsicI->getArgOperand(0);
@@ -2148,85 +2141,7 @@ bool GVNPass::processAssumeIntrinsic(AssumeInst *IntrinsicI) {
}
Constant *True = ConstantInt::getTrue(V->getContext());
- bool Changed = false;
-
- for (BasicBlock *Successor : successors(IntrinsicI->getParent())) {
- BasicBlockEdge Edge(IntrinsicI->getParent(), Successor);
-
- // This property is only true in dominated successors, propagateEquality
- // will check dominance for us.
- Changed |= propagateEquality(V, True, Edge, false);
- }
-
- // We can replace assume value with true, which covers cases like this:
- // call void @llvm.assume(i1 %cmp)
- // br i1 %cmp, label %bb1, label %bb2 ; will change %cmp to true
- ReplaceOperandsWithMap[V] = True;
-
- // Similarly, after assume(!NotV) we know that NotV == false.
- Value *NotV;
- if (match(V, m_Not(m_Value(NotV))))
- ReplaceOperandsWithMap[NotV] = ConstantInt::getFalse(V->getContext());
-
- // If we find an equality fact, canonicalize all dominated uses in this block
- // to one of the two values. We heuristically choice the "oldest" of the
- // two where age is determined by value number. (Note that propagateEquality
- // above handles the cross block case.)
- //
- // Key case to cover are:
- // 1)
- // %cmp = fcmp oeq float 3.000000e+00, %0 ; const on lhs could happen
- // call void @llvm.assume(i1 %cmp)
- // ret float %0 ; will change it to ret float 3.000000e+00
- // 2)
- // %load = load float, float* %addr
- // %cmp = fcmp oeq float %load, %0
- // call void @llvm.assume(i1 %cmp)
- // ret float %load ; will change it to ret float %0
- if (auto *CmpI = dyn_cast<CmpInst>(V)) {
- if (CmpI->isEquivalence()) {
- Value *CmpLHS = CmpI->getOperand(0);
- Value *CmpRHS = CmpI->getOperand(1);
- // Heuristically pick the better replacement -- the choice of heuristic
- // isn't terribly important here, but the fact we canonicalize on some
- // replacement is for exposing other simplifications.
- // TODO: pull this out as a helper function and reuse w/ existing
- // (slightly different) logic.
- if (isa<Constant>(CmpLHS) && !isa<Constant>(CmpRHS))
- std::swap(CmpLHS, CmpRHS);
- if (!isa<Instruction>(CmpLHS) && isa<Instruction>(CmpRHS))
- std::swap(CmpLHS, CmpRHS);
- if ((isa<Argument>(CmpLHS) && isa<Argument>(CmpRHS)) ||
- (isa<Instruction>(CmpLHS) && isa<Instruction>(CmpRHS))) {
- // Move the 'oldest' value to the right-hand side, using the value
- // number as a proxy for age.
- uint32_t LVN = VN.lookupOrAdd(CmpLHS);
- uint32_t RVN = VN.lookupOrAdd(CmpRHS);
- if (LVN < RVN)
- std::swap(CmpLHS, CmpRHS);
- }
-
- // Handle degenerate case where we either haven't pruned a dead path or a
- // removed a trivial assume yet.
- if (isa<Constant>(CmpLHS) && isa<Constant>(CmpRHS))
- return Changed;
-
- LLVM_DEBUG(dbgs() << "Replacing dominated uses of "
- << *CmpLHS << " with "
- << *CmpRHS << " in block "
- << IntrinsicI->getParent()->getName() << "\n");
-
- // Setup the replacement map - this handles uses within the same block.
- if (hasUsersIn(CmpLHS, IntrinsicI->getParent()))
- ReplaceOperandsWithMap[CmpLHS] = CmpRHS;
-
- // NOTE: The non-block local cases are handled by the call to
- // propagateEquality above; this block is just about handling the block
- // local cases. TODO: There's a bunch of logic in propagateEqualiy which
- // isn't duplicated for the block local case, can we share it somehow?
- }
- }
- return Changed;
+ return propagateEquality(V, True, IntrinsicI);
}
static void patchAndReplaceAllUsesWith(Instruction *I, Value *Repl) {
@@ -2496,39 +2411,28 @@ void GVNPass::assignBlockRPONumber(Function &F) {
InvalidBlockRPONumbers = false;
}
-bool GVNPass::replaceOperandsForInBlockEquality(Instruction *Instr) const {
- bool Changed = false;
- for (unsigned OpNum = 0; OpNum < Instr->getNumOperands(); ++OpNum) {
- Use &Operand = Instr->getOperandUse(OpNum);
- auto It = ReplaceOperandsWithMap.find(Operand.get());
- if (It != ReplaceOperandsWithMap.end()) {
- const DataLayout &DL = Instr->getDataLayout();
- if (!canReplacePointersInUseIfEqual(Operand, It->second, DL))
- continue;
-
- LLVM_DEBUG(dbgs() << "GVN replacing: " << *Operand << " with "
- << *It->second << " in instruction " << *Instr << '\n');
- Instr->setOperand(OpNum, It->second);
- Changed = true;
- }
- }
- return Changed;
-}
-
-/// The given values are known to be equal in every block
+/// The given values are known to be equal in every use
/// dominated by 'Root'. Exploit this, for example by replacing 'LHS' with
/// 'RHS' everywhere in the scope. Returns whether a change was made.
/// If DominatesByEdge is false, then it means that we will propagate the RHS
/// value starting from the end of Root.Start.
-bool GVNPass::propagateEquality(Value *LHS, Value *RHS,
- const BasicBlockEdge &Root,
- bool DominatesByEdge) {
+bool GVNPass::propagateEquality(
+ Value *LHS, Value *RHS,
+ const std::variant<BasicBlockEdge, Instruction *> &Root) {
SmallVector<std::pair<Value*, Value*>, 4> Worklist;
Worklist.push_back(std::make_pair(LHS, RHS));
bool Changed = false;
- // For speed, compute a conservative fast approximation to
- // DT->dominates(Root, Root.getEnd());
- const bool RootDominatesEnd = isOnlyReachableViaThisEdge(Root, DT);
+ SmallVector<const BasicBlock *> DominatedBlocks;
+ if (const BasicBlockEdge *Edge = std::get_if<BasicBlockEdge>(&Root)) {
+ // For speed, compute a conservative fast approximation to
+ // DT->dominates(Root, Root.getEnd());
+ if (isOnlyReachableViaThisEdge(*Edge, DT))
+ DominatedBlocks.push_back(Edge->getEnd());
+ } else {
+ Instruction *I = std::get<Instruction *>(Root);
+ for (const auto *Node : DT->getNode(I->getParent())->children())
+ DominatedBlocks.push_back(Node->getBlock());
+ }
while (!Worklist.empty()) {
std::pair<Value*, Value*> Item = Worklist.pop_back_val();
@@ -2576,9 +2480,9 @@ bool GVNPass::propagateEquality(Value *LHS, Value *RHS,
// using the leader table is about compiling faster, not optimizing better).
// The leader table only tracks basic blocks, not edges. Only add to if we
// have the simple case where the edge dominates the end.
- if (RootDominatesEnd && !isa<Instruction>(RHS) &&
- canReplacePointersIfEqual(LHS, RHS, DL))
- LeaderTable.insert(LVN, RHS, Root.getEnd());
+ if (!isa<Instruction>(RHS) && canReplacePointersIfEqual(LHS, RHS, DL))
+ for (const BasicBlock *BB : DominatedBlocks)
+ LeaderTable.insert(LVN, RHS, BB);
// 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
@@ -2588,12 +2492,14 @@ bool GVNPass::propagateEquality(Value *LHS, Value *RHS,
auto CanReplacePointersCallBack = [&DL](const Use &U, const Value *To) {
return canReplacePointersInUseIfEqual(U, To, DL);
};
- unsigned NumReplacements =
- DominatesByEdge
- ? replaceDominatedUsesWithIf(LHS, RHS, *DT, Root,
- CanReplacePointersCallBack)
- : replaceDominatedUsesWithIf(LHS, RHS, *DT, Root.getStart(),
- CanReplacePointersCallBack);
+ unsigned NumReplacements;
+ if (const BasicBlockEdge *Edge = std::get_if<BasicBlockEdge>(&Root))
+ NumReplacements = replaceDominatedUsesWithIf(
+ LHS, RHS, *DT, *Edge, CanReplacePointersCallBack);
+ else
+ NumReplacements = replaceDominatedUsesWithIf(
+ LHS, RHS, *DT, std::get<Instruction *>(Root),
+ CanReplacePointersCallBack);
if (NumReplacements > 0) {
Changed = true;
@@ -2652,26 +2558,45 @@ bool GVNPass::propagateEquality(Value *LHS, Value *RHS,
// If the number we were assigned was brand new then there is no point in
// looking for an instruction realizing it: there cannot be one!
if (Num < NextNum) {
- Value *NotCmp = findLeader(Root.getEnd(), Num);
- if (NotCmp && isa<Instruction>(NotCmp)) {
- unsigned NumReplacements =
- DominatesByEdge
- ? replaceDominatedUsesWith(NotCmp, NotVal, *DT, Root)
- : replaceDominatedUsesWith(NotCmp, NotVal, *DT,
- Root.getStart());
- Changed |= NumReplacements > 0;
- NumGVNEqProp += NumReplacements;
- // Cached information for anything that uses NotCmp will be invalid.
- if (MD)
- MD->invalidateCachedPointerInfo(NotCmp);
+ for (const auto &Entry : LeaderTable.getLeaders(Num)) {
+ // Only look at leaders that either dominate the start of the edge,
+ // or are dominated by the end. This check is not necessary for
+ // correctness, it only discards cases for which the following
+ // use replacement will not work anyway.
+ if (const BasicBlockEdge *Edge = std::get_if<BasicBlockEdge>(&Root)) {
+ if (!DT->dominates(Entry.BB, Edge->getStart()) &&
+ !DT->dominates(Edge->getEnd(), Entry.BB))
+ continue;
+ } else {
+ auto *InstBB = std::get<Instruction *>(Root)->getParent();
+ if (!DT->dominates(Entry.BB, InstBB) &&
+ !DT->dominates(InstBB, Entry.BB))
+ continue;
+ }
+
+ Value *NotCmp = Entry.Val;
+ if (NotCmp && isa<Instruction>(NotCmp)) {
+ unsigned NumReplacements;
+ if (const BasicBlockEdge *Edge = std::get_if<BasicBlockEdge>(&Root))
+ NumReplacements =
+ replaceDominatedUsesWith(NotCmp, NotVal, *DT, *Edge);
+ else
+ NumReplacements = replaceDominatedUsesWith(
+ NotCmp, NotVal, *DT, std::get<Instruction *>(Root));
+ Changed |= NumReplacements > 0;
+ NumGVNEqProp += NumReplacements;
+ // Cached information for anything that uses NotCmp will be invalid.
+ if (MD)
+ MD->invalidateCachedPointerInfo(NotCmp);
+ }
}
}
// Ensure that any instruction in scope that gets the "A < B" value number
// is replaced with false.
// The leader table only tracks basic blocks, not edges. Only add to if we
// have the simple case where the edge dominates the end.
- if (RootDominatesEnd)
- LeaderTable.insert(Num, NotVal, Root.getEnd());
+ for (const BasicBlock *BB : DominatedBlocks)
+ LeaderTable.insert(Num, NotVal, BB);
continue;
}
@@ -2755,11 +2680,11 @@ bool GVNPass::processInstruction(Instruction *I) {
Value *TrueVal = ConstantInt::getTrue(TrueSucc->getContext());
BasicBlockEdge TrueE(Parent, TrueSucc);
- Changed |= propagateEquality(BranchCond, TrueVal, TrueE, true);
+ Changed |= propagateEquality(BranchCond, TrueVal, TrueE);
Value *FalseVal = ConstantInt::getFalse(FalseSucc->getContext());
BasicBlockEdge FalseE(Parent, FalseSucc);
- Changed |= propagateEquality(BranchCond, FalseVal, FalseE, true);
+ Changed |= propagateEquality(BranchCond, FalseVal, FalseE);
return Changed;
}
@@ -2780,7 +2705,7 @@ bool GVNPass::processInstruction(Instruction *I) {
// If there is only a single edge, propagate the case value into it.
if (SwitchEdges.lookup(Dst) == 1) {
BasicBlockEdge E(Parent, Dst);
- Changed |= propagateEquality(SwitchCond, Case.getCaseValue(), E, true);
+ Changed |= propagateEquality(SwitchCond, Case.getCaseValue(), E);
}
}
return Changed;
@@ -2908,8 +2833,6 @@ bool GVNPass::processBlock(BasicBlock *BB) {
if (DeadBlocks.count(BB))
return false;
- // Clearing map before every BB because it can be used only for single BB.
- ReplaceOperandsWithMap.clear();
bool ChangedFunction = false;
// Since we may not have visited the input blocks of the phis, we can't
@@ -2921,11 +2844,8 @@ bool GVNPass::processBlock(BasicBlock *BB) {
for (PHINode *PN : PHINodesToRemove) {
removeInstruction(PN);
}
- for (Instruction &Inst : make_early_inc_range(*BB)) {
- if (!ReplaceOperandsWithMap.empty())
- ChangedFunction |= replaceOperandsForInBlockEquality(&Inst);
+ for (Instruction &Inst : make_early_inc_range(*BB))
ChangedFunction |= processInstruction(&Inst);
- }
return ChangedFunction;
}
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 21b2652d04120..c21f387099afb 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3239,6 +3239,13 @@ unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To,
return ::replaceDominatedUsesWith(From, To, Dominates);
}
+unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To,
+ DominatorTree &DT,
+ const Instruction *I) {
+ auto Dominates = [&](const Use &U) { return DT.dominates(I, U); };
+ return ::replaceDominatedUsesWith(From, To, Dominates);
+}
+
unsigned llvm::replaceDominatedUsesWithIf(
Value *From, Value *To, DominatorTree &DT, const BasicBlockEdge &Root,
function_ref<bool(const Use &U, const Value *To)> ShouldReplace) {
@@ -3257,6 +3264,15 @@ unsigned llvm::replaceDominatedUsesWithIf(
return ::replaceDominatedUsesWith(From, To, DominatesAndShouldReplace);
}
+unsigned llvm::replaceDominatedUsesWithIf(
+ Value *From, Value *To, DominatorTree &DT, const Instruction *I,
+ function_ref<bool(const Use &U, const Value *To)> ShouldReplace) {
+ auto DominatesAndShouldReplace = [&](const Use &U) {
+ return DT.dominates(I, U) && ShouldReplace(U, To);
+ };
+ return ::replaceDominatedUsesWith(From, To, DominatesAndShouldReplace);
+}
+
bool llvm::callsGCLeafFunction(const CallBase *Call,
const TargetLibraryInfo &TLI) {
// Check if the function is specifically marked as a gc leaf function.
diff --git a/llvm/test/Transforms/GVN/condprop.ll b/llvm/test/Transforms/GVN/condprop.ll
index eb2a9f1e847c4..57bd2f3487567 100644
--- a/llvm/test/Transforms/GVN/condprop.ll
+++ b/llvm/test/Transforms/GVN/condprop.ll
@@ -360,7 +360,7 @@ define i1 @test6_phi2(i1 %c, i32 %x, i32 %y) {
; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[X]], [[Y]]
; CHECK-NEXT: br i1 [[CMP]], label [[BB2]], label [[BB3:%.*]]
; CHECK: bb2:
-; CHECK-NEXT: [[PHI:%.*]] = phi i1 [ [[CMP_NOT]], [[BB1]] ], [ true, [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[PHI:%.*]] = phi i1 [ false, [[BB1]] ], [ true, [[ENTRY:%.*]] ]
; CHECK-NEXT: ret i1 [[PHI]]
; CHECK: bb3:
; CHECK-NEXT: ret i1 false
|
Ping |
LLVM_ABI unsigned replaceDominatedUsesWith(Value *From, Value *To, | ||
DominatorTree &DT, | ||
const BasicBlock *BB); | ||
LLVM_ABI unsigned replaceDominatedUsesWith(Value *From, Value *To, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to document the variant with I
llvm/lib/Transforms/Scalar/GVN.cpp
Outdated
/// The given values are known to be equal in every use | ||
/// dominated by 'Root'. Exploit this, for example by replacing 'LHS' with | ||
/// 'RHS' everywhere in the scope. Returns whether a change was made. | ||
/// If DominatesByEdge is false, then it means that we will propagate the RHS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DominatesByEdge
has been removed
Value *From, Value *To, DominatorTree &DT, const BasicBlock *BB, | ||
function_ref<bool(const Use &U, const Value *To)> ShouldReplace); | ||
|
||
LLVM_ABI unsigned replaceDominatedUsesWithIf( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would probably be good to also add a brief comment for this variant
GVN currently has two different implementation of equality propagation. One of them is used for branch conditions (dominating an edge), which performs replacements across multiple blocks. This is also used for assumes to handle uses outside the current block. However, uses inside the block are handled using a completely separate implementation, which involves populating a replacement map and then checking it for individual instructions during normal GVN. While this approach generally makes sense, it is kind of pointless if we already do a use walk to handle the cross-block case anyway. This PR generalizes propagateEquality() to accept either a BasicBlockEdge or an Instruction* and replace dominated users. This removes the need for special handling of uses in the same block for assumes, as they're covered by instruction dominance.
6d00400
to
da94dcc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
GVN currently has two different implementation of equality propagation. One of them is used for branch conditions (dominating an edge), which performs replacements across multiple blocks. This is also used for assumes to handle uses outside the current block. However, uses inside the block are handled using a completely separate implementation, which involves populating a replacement map and then checking it for individual instructions during normal GVN. While this approach generally makes sense, it is kind of pointless if we already do a use walk to handle the cross-block case anyway. This PR generalizes propagateEquality() to accept either a BasicBlockEdge or an Instruction* and replace dominated users. This removes the need for special handling of uses in the same block for assumes, as they're covered by instruction dominance, and ensures that both implementations do not go out of sync.
GVN currently has two different implementation of equality propagation. One of them is used for branch conditions (dominating an edge), which performs replacements across multiple blocks. This is also used for assumes to handle uses outside the current block.
However, uses inside the block are handled using a completely separate implementation, which involves populating a replacement map and then checking it for individual instructions during normal GVN. While this approach generally makes sense, it is kind of pointless if we already do a use walk to handle the cross-block case anyway.
This PR generalizes propagateEquality() to accept either a BasicBlockEdge or an Instruction* and replace dominated users. This removes the need for special handling of uses in the same block for assumes, as they're covered by instruction dominance, and ensures that both implementations do not go out of sync.