[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Use `DominanceInfo` in dialect conversion (PR #120746)

[via llvm-branch-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

In the dialect conversion driver, use `DominanceInfo` to compute a suitable insertion point for N:1 source materializations.

Note: This PR is related to #<!-- -->114940, but I could't reopen it.


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


2 Files Affected:

- (modified) mlir/include/mlir/IR/Dominance.h (+23) 
- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+19-51) 


``````````diff
diff --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h
index 63504cad211a4d..9e1254c1dfe1e1 100644
--- a/mlir/include/mlir/IR/Dominance.h
+++ b/mlir/include/mlir/IR/Dominance.h
@@ -187,6 +187,17 @@ 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;
+
+  bool properlyDominates(Block *aBlock, Block::iterator aIt, Block *bBlock,
+                         Block::iterator bIt, bool enclosingOk = true) const {
+    return super::properlyDominatesImpl(aBlock, aIt, bBlock, bIt, enclosingOk);
+  }
+
+  bool dominates(Block *aBlock, Block::iterator aIt, Block *bBlock,
+                 Block::iterator bIt, bool enclosingOk = true) const {
+    return (aBlock == bBlock && aIt == bIt) ||
+           super::properlyDominatesImpl(aBlock, aIt, bBlock, bIt, enclosingOk);
+  }
 };
 
 /// A class for computing basic postdominance information.
@@ -210,6 +221,18 @@ class PostDominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/true> {
   bool postDominates(Block *a, Block *b) const {
     return a == b || properlyPostDominates(a, b);
   }
+
+  bool properlyPostDominates(Block *aBlock, Block::iterator aIt, Block *bBlock,
+                             Block::iterator bIt,
+                             bool enclosingOk = true) const {
+    return super::properlyDominatesImpl(aBlock, aIt, bBlock, bIt, enclosingOk);
+  }
+
+  bool postDominates(Block *aBlock, Block::iterator aIt, Block *bBlock,
+                     Block::iterator bIt, bool enclosingOk = true) const {
+    return (aBlock == bBlock && aIt == bIt) ||
+           super::properlyDominatesImpl(aBlock, aIt, bBlock, bIt, enclosingOk);
+  }
 };
 
 } // namespace mlir
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index ea169a1df42b6a..f228cd1fd11f48 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -54,55 +54,6 @@ static void logFailure(llvm::ScopedPrinter &os, StringRef fmt, Args &&...args) {
   });
 }
 
-/// Given two insertion points in the same block, choose the later one.
-static OpBuilder::InsertPoint
-chooseLaterInsertPointInBlock(OpBuilder::InsertPoint a,
-                              OpBuilder::InsertPoint b) {
-  assert(a.getBlock() == b.getBlock() && "expected same block");
-  Block *block = a.getBlock();
-  if (a.getPoint() == block->begin())
-    return b;
-  if (b.getPoint() == block->begin())
-    return a;
-  if (a.getPoint()->isBeforeInBlock(&*b.getPoint()))
-    return b;
-  return a;
-}
-
-/// Helper function that chooses the insertion point among the two given ones
-/// that is later.
-// TODO: Extend DominanceInfo API to work with block iterators.
-static OpBuilder::InsertPoint chooseLaterInsertPoint(OpBuilder::InsertPoint a,
-                                                     OpBuilder::InsertPoint b) {
-  // Case 1: Same block.
-  if (a.getBlock() == b.getBlock())
-    return chooseLaterInsertPointInBlock(a, b);
-
-  // Case 2: Different block, but same region.
-  if (a.getBlock()->getParent() == b.getBlock()->getParent()) {
-    DominanceInfo domInfo;
-    if (domInfo.properlyDominates(a.getBlock(), b.getBlock()))
-      return b;
-    if (domInfo.properlyDominates(b.getBlock(), a.getBlock()))
-      return a;
-    // Neither of the two blocks dominante each other.
-    llvm_unreachable("unable to find valid insertion point");
-  }
-
-  // Case 3: b's region contains a: choose a.
-  if (Operation *aParent = b.getBlock()->getParent()->findAncestorOpInRegion(
-          *a.getPoint()->getParentOp()))
-    return a;
-
-  // Case 4: a's region contains b: choose b.
-  if (Operation *bParent = a.getBlock()->getParent()->findAncestorOpInRegion(
-          *b.getPoint()->getParentOp()))
-    return b;
-
-  // Neither of the two operations contain each other.
-  llvm_unreachable("unable to find valid insertion point");
-}
-
 /// Helper function that computes an insertion point where the given value is
 /// defined and can be used without a dominance violation.
 static OpBuilder::InsertPoint computeInsertPoint(Value value) {
@@ -117,9 +68,26 @@ static OpBuilder::InsertPoint computeInsertPoint(Value value) {
 /// defined and can be used without a dominance violation.
 static OpBuilder::InsertPoint computeInsertPoint(ArrayRef<Value> vals) {
   assert(!vals.empty() && "expected at least one value");
+  DominanceInfo domInfo;
   OpBuilder::InsertPoint pt = computeInsertPoint(vals.front());
-  for (Value v : vals.drop_front())
-    pt = chooseLaterInsertPoint(pt, computeInsertPoint(v));
+  for (Value v : vals.drop_front()) {
+    // Choose the "later" insertion point.
+    OpBuilder::InsertPoint nextPt = computeInsertPoint(v);
+    if (domInfo.dominates(pt.getBlock(), pt.getPoint(), nextPt.getBlock(),
+                          nextPt.getPoint())) {
+      // pt is before nextPt => choose nextPt.
+      pt = nextPt;
+    } else {
+#ifndef NDEBUG
+      // nextPt should be before pt => choose pt.
+      // If pt, nextPt are no dominance relationship, then there is no valid
+      // insertion point at which all given values are defined.
+      bool dom = domInfo.dominates(nextPt.getBlock(), nextPt.getPoint(),
+                                   pt.getBlock(), pt.getPoint());
+      assert(dom && "unable to find valid insertion point");
+#endif // NDEBUG
+    }
+  }
   return pt;
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/120746