[PATCH] D120811: [Dominators] Don't treat multi-edge as dominating
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 2 06:33:51 PST 2022
nikic created this revision.
nikic added reviewers: kuhar, asbirlea, mkazantsev.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
In https://github.com/llvm/llvm-project/blob/d2edca6276d1715a02d1144eae577b3d79673d67/llvm/include/llvm/IR/Dominators.h#L201-L202 we say:
> If BBE is not a unique edge between start and end of the edge, it can
> never dominate the use.
And the code starting at https://github.com/llvm/llvm-project/blob/d2edca6276d1715a02d1144eae577b3d79673d67/llvm/lib/IR/Dominators.cpp#L222 does handle it this way.
However, we also have two special cases for edge-phi domination and edge-edge domination, where we consider a multi-edge to be dominating. This patch makes them non-dominating, as we don't know which edge of the multi-edge we're querying.
I'm somewhat unsure about this change, because the resulting semantics could be considered somewhat odd (an edge does not dominate itself if it is a multi-edge), but I think they match the documentation and are at least self-consistent.
https://reviews.llvm.org/D120811
Files:
llvm/lib/IR/Dominators.cpp
llvm/unittests/IR/DominatorTreeTest.cpp
Index: llvm/unittests/IR/DominatorTreeTest.cpp
===================================================================
--- llvm/unittests/IR/DominatorTreeTest.cpp
+++ llvm/unittests/IR/DominatorTreeTest.cpp
@@ -1072,6 +1072,50 @@
});
}
+TEST(DominatorTree, SwitchEdgeDomination) {
+ StringRef ModuleString =
+ "define i32 @f(i32 %i, i32 *%p) {\n"
+ "bb0:\n"
+ " store i32 %i, i32 *%p\n"
+ " switch i32 %i, label %bb2 [\n"
+ " i32 0, label %bb1\n"
+ " i32 1, label %bb1\n"
+ " ]\n"
+ " bb1:\n"
+ " %phi = phi i32 [ 2, %bb0 ], [ 3, %bb0 ]\n"
+ " ret i32 1\n"
+ " bb2:\n"
+ " ret i32 4\n"
+ "}\n";
+
+ // Parse the module.
+ LLVMContext Context;
+ std::unique_ptr<Module> M = makeLLVMModule(Context, ModuleString);
+
+ runWithDomTree(*M, "f",
+ [&](Function &F, DominatorTree *DT, PostDominatorTree *PDT) {
+ Function::iterator FI = F.begin();
+
+ BasicBlock *BB0 = &*FI++;
+ BasicBlock *BB1 = &*FI++;
+ BasicBlock *BB2 = &*FI++;
+ Instruction &Phi = *BB1->begin();
+
+ BasicBlockEdge E01(BB0, BB1);
+ BasicBlockEdge E02(BB0, BB2);
+
+ EXPECT_TRUE(DT->dominates(E02, E02));
+ EXPECT_FALSE(DT->dominates(E02, E01));
+ EXPECT_FALSE(DT->dominates(E01, E02));
+ // Does not dominate, because it is a multi-edge.
+ EXPECT_FALSE(DT->dominates(E01, E01));
+
+ // Similarly, multi-edge phi uses are not dominated.
+ EXPECT_FALSE(DT->dominates(E01, Phi.getOperandUse(0)));
+ EXPECT_FALSE(DT->dominates(E01, Phi.getOperandUse(1)));
+ });
+}
+
TEST(DominatorTree, ValueDomination) {
StringRef ModuleString = R"(
@foo = global i8 0
Index: llvm/lib/IR/Dominators.cpp
===================================================================
--- llvm/lib/IR/Dominators.cpp
+++ llvm/lib/IR/Dominators.cpp
@@ -261,7 +261,7 @@
PHINode *PN = dyn_cast<PHINode>(UserInst);
if (PN && PN->getParent() == BBE.getEnd() &&
PN->getIncomingBlock(U) == BBE.getStart())
- return true;
+ return BBE.isSingleEdge();
// Otherwise use the edge-dominates-block query, which
// handles the crazy critical edge cases properly.
@@ -352,7 +352,7 @@
bool DominatorTree::dominates(const BasicBlockEdge &BBE1,
const BasicBlockEdge &BBE2) const {
if (BBE1.getStart() == BBE2.getStart() && BBE1.getEnd() == BBE2.getEnd())
- return true;
+ return BBE1.isSingleEdge();
return dominates(BBE1, BBE2.getStart());
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D120811.412393.patch
Type: text/x-patch
Size: 2495 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220302/1210ff07/attachment.bin>
More information about the llvm-commits
mailing list