[PATCH] D120811: [Dominators] Don't treat multi-edge as dominating

[Nikita Popov via Phabricator via llvm-commits]

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>