[mlir][IR] DominanceInfo: Deduplicate properlyDominates implementation

The implementations of `DominanceInfo::properlyDominates` and `PostDominanceInfo::properlyPostDominates` are almost identical: only one line of code is different. Define the function in `DominanceInfoBase` to avoid the code duplication.

Note: This commit is not marked as NFC because `PostDominanceInfo::properlyPostDominates` now also has an `enclosingOpOk` argument.

Author: matthias-springer

mlir/include/mlir/IR/Dominance.h

@@ -113,8 +113,12 @@ class DominanceInfoBase {
   llvm::PointerIntPair<DomTree *, 1, bool>
   getDominanceInfo(Region *region, bool needsDomTree) const;
 
-  /// Return true if the specified block A properly dominates block B.
-  bool properlyDominates(Block *a, Block *b) const;
+  /// Return "true" if the specified block A properly (post)dominates block B.
+  bool properlyDominatesImpl(Block *a, Block *b) const;
+
+  /// Return "true" if the specified op A properly (post)dominates op B.
+  bool properlyDominatesImpl(Operation *a, Operation *b,
+                             bool enclosingOpOk = true) const;
 
   /// A mapping of regions to their base dominator tree and a cached
   /// "hasSSADominance" bit. This map does not contain dominator trees for
@@ -147,7 +151,9 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
   /// The `enclosingOpOk` flag says whether we should return true if the B op
   /// is enclosed by a region on A.
   bool properlyDominates(Operation *a, Operation *b,
-                         bool enclosingOpOk = true) const;
+                         bool enclosingOpOk = true) const {
+    return super::properlyDominatesImpl(a, b, enclosingOpOk);
+  }
 
   /// Return true if operation A dominates operation B, i.e. if A and B are the
   /// same operation or A properly dominates B.
@@ -183,7 +189,7 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
   /// dominance" of ops, the single block is considered to properly dominate
   /// itself in a graph region.
   bool properlyDominates(Block *a, Block *b) const {
-    return super::properlyDominates(a, b);
+    return super::properlyDominatesImpl(a, b);
   }
 };
 
@@ -193,7 +199,10 @@ class PostDominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/true> {
   using super::super;
 
   /// Return true if operation A properly postdominates operation B.
-  bool properlyPostDominates(Operation *a, Operation *b) const;
+  bool properlyPostDominates(Operation *a, Operation *b,
+                             bool enclosingOpOk = true) const {
+    return super::properlyDominatesImpl(a, b, enclosingOpOk);
+  }
 
   /// Return true if operation A postdominates operation B.
   bool postDominates(Operation *a, Operation *b) const {
@@ -202,7 +211,7 @@ class PostDominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/true> {
 
   /// Return true if the specified block A properly postdominates block B.
   bool properlyPostDominates(Block *a, Block *b) const {
-    return super::properlyDominates(a, b);
+    return super::properlyDominatesImpl(a, b);
   }
 
   /// Return true if the specified block A postdominates block B.

mlir/lib/IR/Dominance.cpp

@@ -215,7 +215,8 @@ DominanceInfoBase<IsPostDom>::findNearestCommonDominator(Block *a,
 
 /// Return true if the specified block A properly dominates block B.
 template <bool IsPostDom>
-bool DominanceInfoBase<IsPostDom>::properlyDominates(Block *a, Block *b) const {
+bool DominanceInfoBase<IsPostDom>::properlyDominatesImpl(Block *a,
+                                                         Block *b) const {
   assert(a && b && "null blocks not allowed");
 
   // A block dominates, but does not properly dominate, itself unless this
@@ -243,51 +244,29 @@ bool DominanceInfoBase<IsPostDom>::properlyDominates(Block *a, Block *b) const {
   return getDomTree(regionA).properlyDominates(a, b);
 }
 
-/// Return true if the specified block is reachable from the entry block of
-/// its region.
 template <bool IsPostDom>
-bool DominanceInfoBase<IsPostDom>::isReachableFromEntry(Block *a) const {
-  // If this is the first block in its region, then it is obviously reachable.
-  Region *region = a->getParent();
-  if (&region->front() == a)
-    return true;
-
-  // Otherwise this is some block in a multi-block region.  Check DomTree.
-  return getDomTree(region).isReachableFromEntry(a);
-}
-
-template class detail::DominanceInfoBase</*IsPostDom=*/true>;
-template class detail::DominanceInfoBase</*IsPostDom=*/false>;
-
-//===----------------------------------------------------------------------===//
-// DominanceInfo
-//===----------------------------------------------------------------------===//
-
-/// Return true if operation `a` properly dominates operation `b`.  The
-/// 'enclosingOpOk' flag says whether we should return true if the `b` op is
-/// enclosed by a region on 'a'.
-bool DominanceInfo::properlyDominates(Operation *a, Operation *b,
-                                      bool enclosingOpOk) const {
+bool DominanceInfoBase<IsPostDom>::properlyDominatesImpl(
+    Operation *a, Operation *b, bool enclosingOpOk) const {
   Block *aBlock = a->getBlock(), *bBlock = b->getBlock();
   assert(aBlock && bBlock && "operations must be in a block");
 
-  // An operation dominates, but does not properly dominate, itself unless this
-  // is a graph region.
+  // An operation (pos)dominates, but does not properly (pos)dominate, itself
+  // unless this is a graph region.
   if (a == b)
     return !hasSSADominance(aBlock);
 
   // If these ops are in different regions, then normalize one into the other.
   Region *aRegion = aBlock->getParent();
   if (aRegion != bBlock->getParent()) {
     // Scoot up b's region tree until we find an operation in A's region that
-    // encloses it.  If this fails, then we know there is no post-dom relation.
+    // encloses it.  If this fails, then we know there is no (post)dom relation.
     b = aRegion ? aRegion->findAncestorOpInRegion(*b) : nullptr;
     if (!b)
       return false;
     bBlock = b->getBlock();
     assert(bBlock->getParent() == aRegion);
 
-    // If 'a' encloses 'b', then we consider it to dominate.
+    // If 'a' encloses 'b', then we consider it to (post)dominate.
     if (a == b && enclosingOpOk)
       return true;
   }
@@ -297,17 +276,39 @@ bool DominanceInfo::properlyDominates(Operation *a, Operation *b,
     // Dominance changes based on the region type. In a region with SSA
     // dominance, uses inside the same block must follow defs. In other
     // regions kinds, uses and defs can come in any order inside a block.
-    if (hasSSADominance(aBlock)) {
-      // If the blocks are the same, then check if b is before a in the block.
+    if (!hasSSADominance(aBlock))
+      return true;
+    if constexpr (IsPostDom) {
+      return b->isBeforeInBlock(a);
+    } else {
       return a->isBeforeInBlock(b);
     }
-    return true;
   }
 
   // If the blocks are different, use DomTree to resolve the query.
   return getDomTree(aRegion).properlyDominates(aBlock, bBlock);
 }
 
+/// Return true if the specified block is reachable from the entry block of
+/// its region.
+template <bool IsPostDom>
+bool DominanceInfoBase<IsPostDom>::isReachableFromEntry(Block *a) const {
+  // If this is the first block in its region, then it is obviously reachable.
+  Region *region = a->getParent();
+  if (&region->front() == a)
+    return true;
+
+  // Otherwise this is some block in a multi-block region.  Check DomTree.
+  return getDomTree(region).isReachableFromEntry(a);
+}
+
+template class detail::DominanceInfoBase</*IsPostDom=*/true>;
+template class detail::DominanceInfoBase</*IsPostDom=*/false>;
+
+//===----------------------------------------------------------------------===//
+// DominanceInfo
+//===----------------------------------------------------------------------===//
+
 /// Return true if the `a` value properly dominates operation `b`, i.e if the
 /// operation that defines `a` properlyDominates `b` and the operation that
 /// defines `a` does not contain `b`.
@@ -321,49 +322,3 @@ bool DominanceInfo::properlyDominates(Value a, Operation *b) const {
   // `b`, but `a` does not itself enclose `b` in one of its regions.
   return properlyDominates(a.getDefiningOp(), b, /*enclosingOpOk=*/false);
 }
-
-//===----------------------------------------------------------------------===//
-// PostDominanceInfo
-//===----------------------------------------------------------------------===//
-
-/// Returns true if statement 'a' properly postdominates statement b.
-bool PostDominanceInfo::properlyPostDominates(Operation *a,
-                                              Operation *b) const {
-  auto *aBlock = a->getBlock(), *bBlock = b->getBlock();
-  assert(aBlock && bBlock && "operations must be in a block");
-
-  // An instruction postDominates, but does not properlyPostDominate, itself
-  // unless this is a graph region.
-  if (a == b)
-    return !hasSSADominance(aBlock);
-
-  // If these ops are in different regions, then normalize one into the other.
-  Region *aRegion = aBlock->getParent();
-  if (aRegion != bBlock->getParent()) {
-    // Scoot up b's region tree until we find an operation in A's region that
-    // encloses it.  If this fails, then we know there is no post-dom relation.
-    b = aRegion ? aRegion->findAncestorOpInRegion(*b) : nullptr;
-    if (!b)
-      return false;
-    bBlock = b->getBlock();
-    assert(bBlock->getParent() == aRegion);
-
-    // If 'a' encloses 'b', then we consider it to postdominate.
-    if (a == b)
-      return true;
-  }
-
-  // Ok, they are in the same region.  If they are in the same block, check if b
-  // is before a in the block.
-  if (aBlock == bBlock) {
-    // Dominance changes based on the region type.
-    if (hasSSADominance(aBlock)) {
-      // If the blocks are the same, then check if b is before a in the block.
-      return b->isBeforeInBlock(a);
-    }
-    return true;
-  }
-
-  // If the blocks are different, check if a's block post dominates b's.
-  return getDomTree(aRegion).properlyDominates(aBlock, bBlock);
-}