[llvm] eadf295 - [CodeMoverUtils] Use dominator tree level to decide the direction of
Whitney Tsang via llvm-commits
llvm-commits at lists.llvm.org
Wed May 27 11:02:48 PDT 2020
Author: Rithik Sharma
Date: 2020-05-27T18:02:06Z
New Revision: eadf2959567c89bebff153feac873cbc1b71eb04
URL: https://github.com/llvm/llvm-project/commit/eadf2959567c89bebff153feac873cbc1b71eb04
DIFF: https://github.com/llvm/llvm-project/commit/eadf2959567c89bebff153feac873cbc1b71eb04.diff
LOG: [CodeMoverUtils] Use dominator tree level to decide the direction of
code motion
Summary: Currently isSafeToMoveBefore uses DFS numbering for determining
the relative position of instruction and insert point which is not
always correct. This PR proposes the use of Dominator Tree depth for the
same. If a node is at a higher level than the insert point then it is
safe to say that we want to move in the forward direction.
Authored By: RithikSharma
Reviewer: Whitney, nikic, bmahjour, etiotto, fhahn
Reviewed By: Whitney
Subscribers: fhahn, hiraditya, llvm-commits
Tag: LLVM
Differential Revision: https://reviews.llvm.org/D80084
Added:
Modified:
llvm/include/llvm/Analysis/OrderedInstructions.h
llvm/lib/Analysis/OrderedInstructions.cpp
llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/OrderedInstructions.h b/llvm/include/llvm/Analysis/OrderedInstructions.h
index 302509380a03..e380a0045377 100644
--- a/llvm/include/llvm/Analysis/OrderedInstructions.h
+++ b/llvm/include/llvm/Analysis/OrderedInstructions.h
@@ -45,6 +45,12 @@ class OrderedInstructions {
/// or if the first instruction comes before the second in the same basic
/// block.
bool dfsBefore(const Instruction *, const Instruction *) const;
+
+ // Return true if the first instruction comes before the second in the
+ // dominator tree BFS traversal based on the level number of nodes in
+ // dominator tree if they are in
diff erent basic blocks else if the first
+ // instruction comes before the second in the same basic block.
+ bool domTreeLevelBefore(const Instruction *, const Instruction *) const;
};
} // end namespace llvm
diff --git a/llvm/lib/Analysis/OrderedInstructions.cpp b/llvm/lib/Analysis/OrderedInstructions.cpp
index 11ab3e0927d2..98a65f31a36c 100644
--- a/llvm/lib/Analysis/OrderedInstructions.cpp
+++ b/llvm/lib/Analysis/OrderedInstructions.cpp
@@ -43,3 +43,15 @@ bool OrderedInstructions::dfsBefore(const Instruction *InstA,
DomTreeNode *DB = DT->getNode(InstB->getParent());
return DA->getDFSNumIn() < DB->getDFSNumIn();
}
+
+bool OrderedInstructions::domTreeLevelBefore(const Instruction *InstA,
+ const Instruction *InstB) const {
+ // Use ordered basic block in case the 2 instructions are in the same basic
+ // block.
+ if (InstA->getParent() == InstB->getParent())
+ return localDominates(InstA, InstB);
+
+ DomTreeNode *DA = DT->getNode(InstA->getParent());
+ DomTreeNode *DB = DT->getNode(InstB->getParent());
+ return DA->getLevel() < DB->getLevel();
+}
diff --git a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
index 383e749f32c4..4583ff74167a 100644
--- a/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
+++ b/llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
@@ -317,25 +317,20 @@ bool llvm::isSafeToMoveBefore(Instruction &I, Instruction &InsertPoint,
if (!isControlFlowEquivalent(I, InsertPoint, DT, PDT))
return reportInvalidCandidate(I, NotControlFlowEquivalent);
- OrderedInstructions OI(&DT);
- DT.updateDFSNumbers();
- const bool MoveForward = OI.dfsBefore(&I, &InsertPoint);
- if (MoveForward) {
- // When I is being moved forward, we need to make sure the InsertPoint
- // dominates every users. Or else, a user may be using an undefined I.
+ if (!DT.dominates(&InsertPoint, &I))
for (const Use &U : I.uses())
if (auto *UserInst = dyn_cast<Instruction>(U.getUser()))
if (UserInst != &InsertPoint && !DT.dominates(&InsertPoint, U))
return false;
- } else {
- // When I is being moved backward, we need to make sure all its opernads
- // dominates the InsertPoint. Or else, an operand may be undefined for I.
+ if (!DT.dominates(&I, &InsertPoint))
for (const Value *Op : I.operands())
if (auto *OpInst = dyn_cast<Instruction>(Op))
- if (&InsertPoint == OpInst || !OI.dominates(OpInst, &InsertPoint))
+ if (&InsertPoint == OpInst || !DT.dominates(OpInst, &InsertPoint))
return false;
- }
+ OrderedInstructions OI(&DT);
+ DT.updateDFSNumbers();
+ const bool MoveForward = OI.domTreeLevelBefore(&I, &InsertPoint);
Instruction &StartInst = (MoveForward ? I : InsertPoint);
Instruction &EndInst = (MoveForward ? InsertPoint : I);
SmallPtrSet<Instruction *, 10> InstsToCheck;
diff --git a/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp b/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
index bfa269e8f6ee..ceeb4071ab7d 100644
--- a/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
+++ b/llvm/unittests/Transforms/Utils/CodeMoverUtilsTest.cpp
@@ -613,3 +613,39 @@ TEST(CodeMoverUtils, IsSafeToMoveTest3) {
EXPECT_TRUE(isSafeToMoveBefore(*IncInst, *CmpInst, DT, PDT, DI));
});
}
+
+TEST(CodeMoverUtils, IsSafeToMoveTest4) {
+ LLVMContext C;
+
+ std::unique_ptr<Module> M =
+ parseIR(C, R"(define void @foo(i1 %cond, i32 %op0, i32 %op1) {
+ entry:
+ br i1 %cond, label %if.end.first, label %if.then.first
+ if.then.first:
+ %add = add i32 %op0, %op1
+ %user = add i32 %add, 1
+ br label %if.end.first
+ if.end.first:
+ br i1 %cond, label %if.end.second, label %if.then.second
+ if.then.second:
+ %sub_op0 = add i32 %op0, 1
+ %sub = sub i32 %sub_op0, %op1
+ br label %if.end.second
+ if.end.second:
+ ret void
+ })");
+
+ run(*M, "foo",
+ [&](Function &F, DominatorTree &DT, PostDominatorTree &PDT,
+ DependenceInfo &DI) {
+ Instruction *AddInst = getInstructionByName(F, "add");
+ Instruction *SubInst = getInstructionByName(F, "sub");
+
+ // Cannot move as %user uses %add and %sub doesn't dominates %user.
+ EXPECT_FALSE(isSafeToMoveBefore(*AddInst, *SubInst, DT, PDT, DI));
+
+ // Cannot move as %sub_op0 is an operand of %sub and %add doesn't
+ // dominates %sub_op0.
+ EXPECT_FALSE(isSafeToMoveBefore(*SubInst, *AddInst, DT, PDT, DI));
+ });
+}
More information about the llvm-commits
mailing list