[mlir][Transforms][NFC] Use DominanceInfo to compute materialization insertion point
#120746
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-mlir Author: Matthias Springer (matthias-springer) ChangesIn the dialect conversion driver, use 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:
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;
}
|
|
@llvm/pr-subscribers-mlir-core Author: Matthias Springer (matthias-springer) ChangesIn the dialect conversion driver, use 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:
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;
}
|
matthias-springer
changed the title
DominanceInfo in dialect conversionDominanceInfo to compute materialization insertion point
matthias-springer
force-pushed
the
users/matthias-springer/block_dom
branch
from
b484c72 to
077584f
Compare
matthias-springer
requested review from
aartbik,
PeimingLiu,
yinying-lisa-li,
banach-space,
dcaballe,
hanhanW,
nicolasvasilache and
rengolin
as code owners
matthias-springer
force-pushed
the
users/matthias-springer/insert_pt
branch
from
61b5137 to
b6a6f1c
Compare
matthias-springer
force-pushed
the
users/matthias-springer/insert_pt
branch
from
b6a6f1c to
9451dbe
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the dialect conversion driver, use
DominanceInfoto compute a suitable insertion point for N:1 source materializations.Note: This PR is related to #114940, but I could't reopen it.