Skip to content

[mlir][IR] DominanceInfo: Deduplicate properlyDominates implementation #115433

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

Merged

Conversation

matthias-springer
Copy link
Member

@matthias-springer matthias-springer commented Nov 8, 2024

The implementations of DominanceInfo::properlyDominates and PostDominanceInfo::properlyPostDominates are almost identical: only one line of code is different (apart from the missing enclosingOpOk flag). Define the function in DominanceInfoBase to avoid the code duplication.

Also rename the helper in DominanceInfoBase to properlyDominatesImpl.

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

Depends on #115430.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Nov 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2024

@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)

Changes

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.

Also rename the helper in DominanceInfoBase to properlyDominatesImpl.

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

Depends on #115430.


Full diff: https://github.com/llvm/llvm-project/pull/115433.diff

2 Files Affected:

  • (modified) mlir/include/mlir/IR/Dominance.h (+15-6)
  • (modified) mlir/lib/IR/Dominance.cpp (+33-77)
diff --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h
index 15b033ec854fde..933ec09c5ede29 100644
--- a/mlir/include/mlir/IR/Dominance.h
+++ b/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);
+  bool properlyPostDominates(Operation *a, Operation *b,
+                             bool enclosingOpOk = true) {
+    return super::properlyDominatesImpl(a, b, enclosingOpOk);
+  }
 
   /// Return true if operation A postdominates operation B.
   bool postDominates(Operation *a, Operation *b) {
@@ -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) {
-    return super::properlyDominates(a, b);
+    return super::properlyDominatesImpl(a, b);
   }
 
   /// Return true if the specified block A postdominates block B.
diff --git a/mlir/lib/IR/Dominance.cpp b/mlir/lib/IR/Dominance.cpp
index bdd0febfa546e0..406e0f2d62d640 100644
--- a/mlir/lib/IR/Dominance.cpp
+++ 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,36 +244,14 @@ 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);
 
@@ -280,14 +259,14 @@ bool DominanceInfo::properlyDominates(Operation *a, Operation *b,
   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,48 +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) {
-  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);
-}

@llvmbot
Copy link
Member

llvmbot commented Nov 8, 2024

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

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.

Also rename the helper in DominanceInfoBase to properlyDominatesImpl.

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

Depends on #115430.


Full diff: https://github.com/llvm/llvm-project/pull/115433.diff

2 Files Affected:

  • (modified) mlir/include/mlir/IR/Dominance.h (+15-6)
  • (modified) mlir/lib/IR/Dominance.cpp (+33-77)
diff --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h
index 15b033ec854fde..933ec09c5ede29 100644
--- a/mlir/include/mlir/IR/Dominance.h
+++ b/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);
+  bool properlyPostDominates(Operation *a, Operation *b,
+                             bool enclosingOpOk = true) {
+    return super::properlyDominatesImpl(a, b, enclosingOpOk);
+  }
 
   /// Return true if operation A postdominates operation B.
   bool postDominates(Operation *a, Operation *b) {
@@ -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) {
-    return super::properlyDominates(a, b);
+    return super::properlyDominatesImpl(a, b);
   }
 
   /// Return true if the specified block A postdominates block B.
diff --git a/mlir/lib/IR/Dominance.cpp b/mlir/lib/IR/Dominance.cpp
index bdd0febfa546e0..406e0f2d62d640 100644
--- a/mlir/lib/IR/Dominance.cpp
+++ 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,36 +244,14 @@ 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);
 
@@ -280,14 +259,14 @@ bool DominanceInfo::properlyDominates(Operation *a, Operation *b,
   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,48 +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) {
-  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);
-}

matthias-springer added a commit that referenced this pull request Nov 9, 2024
The `properlyDominates` implementations for blocks and ops are very similar. This commit replaces them with a single implementation that operates on block iterators. That implementation can be used to implement both `properlyDominates` variants.

Note: A subsequent commit will add a new public `properlyDominates` overload that accepts block iterators. That functionality can then be used to find a valid insertion point at which a range of values is defined (by utilizing post dominance).

Depends on #115433.
matthias-springer added a commit that referenced this pull request Nov 9, 2024
The `properlyDominates` implementations for blocks and ops are very similar. This commit replaces them with a single implementation that operates on block iterators. That implementation can be used to implement both `properlyDominates` variants.

Note: A subsequent commit will add a new public `properlyDominates` overload that accepts block iterators. That functionality can then be used to find a valid insertion point at which a range of values is defined (by utilizing post dominance).

Depends on #115433.
@matthias-springer matthias-springer force-pushed the users/matthias-springer/minor_dom_cleanup branch from 58fbba4 to f997040 Compare November 10, 2024 06:21
@matthias-springer matthias-springer force-pushed the users/matthias-springer/dominance_deduplicate branch from 26e5b4f to d6ab91c Compare November 10, 2024 06:22
matthias-springer added a commit that referenced this pull request Nov 10, 2024
The `properlyDominates` implementations for blocks and ops are very similar. This commit replaces them with a single implementation that operates on block iterators. That implementation can be used to implement both `properlyDominates` variants.

Note: A subsequent commit will add a new public `properlyDominates` overload that accepts block iterators. That functionality can then be used to find a valid insertion point at which a range of values is defined (by utilizing post dominance).

Depends on #115433.
@matthias-springer matthias-springer force-pushed the users/matthias-springer/minor_dom_cleanup branch from f997040 to 8bc07a5 Compare November 11, 2024 02:44
@matthias-springer matthias-springer force-pushed the users/matthias-springer/dominance_deduplicate branch from d6ab91c to 213a01b Compare November 11, 2024 02:46
matthias-springer added a commit that referenced this pull request Nov 11, 2024
The `properlyDominates` implementations for blocks and ops are very similar. This commit replaces them with a single implementation that operates on block iterators. That implementation can be used to implement both `properlyDominates` variants.

Note: A subsequent commit will add a new public `properlyDominates` overload that accepts block iterators. That functionality can then be used to find a valid insertion point at which a range of values is defined (by utilizing post dominance).

Depends on #115433.
Base automatically changed from users/matthias-springer/minor_dom_cleanup to main November 12, 2024 09:56
…ation

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.
@matthias-springer matthias-springer force-pushed the users/matthias-springer/dominance_deduplicate branch from 213a01b to 4c303e1 Compare November 12, 2024 09:58
@matthias-springer matthias-springer merged commit e65c542 into main Nov 12, 2024
8 checks passed
@matthias-springer matthias-springer deleted the users/matthias-springer/dominance_deduplicate branch November 12, 2024 10:58
matthias-springer added a commit that referenced this pull request Nov 12, 2024
The `properlyDominates` implementations for blocks and ops are very similar. This commit replaces them with a single implementation that operates on block iterators. That implementation can be used to implement both `properlyDominates` variants.

Note: A subsequent commit will add a new public `properlyDominates` overload that accepts block iterators. That functionality can then be used to find a valid insertion point at which a range of values is defined (by utilizing post dominance).

Depends on #115433.
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
…ation (llvm#115433)

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

Also rename the helper in `DominanceInfoBase` to
`properlyDominatesImpl`.

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

Depends on llvm#115430.
matthias-springer added a commit that referenced this pull request Dec 20, 2024
The `properlyDominates` implementations for blocks and ops are very similar. This commit replaces them with a single implementation that operates on block iterators. That implementation can be used to implement both `properlyDominates` variants.

Note: A subsequent commit will add a new public `properlyDominates` overload that accepts block iterators. That functionality can then be used to find a valid insertion point at which a range of values is defined (by utilizing post dominance).

Depends on #115433.
matthias-springer added a commit that referenced this pull request Jan 3, 2025
The `properlyDominates` implementations for blocks and ops are very similar. This commit replaces them with a single implementation that operates on block iterators. That implementation can be used to implement both `properlyDominates` variants.

Note: A subsequent commit will add a new public `properlyDominates` overload that accepts block iterators. That functionality can then be used to find a valid insertion point at which a range of values is defined (by utilizing post dominance).

Depends on #115433.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants