-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[mlir][IR][NFC] DominanceInfo
: Minor code cleanups
#115430
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
matthias-springer
merged 1 commit into
main
from
users/matthias-springer/minor_dom_cleanup
Nov 12, 2024
Merged
[mlir][IR][NFC] DominanceInfo
: Minor code cleanups
#115430
matthias-springer
merged 1 commit into
main
from
users/matthias-springer/minor_dom_cleanup
Nov 12, 2024
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-core @llvm/pr-subscribers-mlir Author: Matthias Springer (matthias-springer) ChangesRemove Depends on #115413. Full diff: https://github.com/llvm/llvm-project/pull/115430.diff 2 Files Affected:
diff --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h
index 95c99bd59f7b2f..15b033ec854fde 100644
--- a/mlir/include/mlir/IR/Dominance.h
+++ b/mlir/include/mlir/IR/Dominance.h
@@ -147,9 +147,7 @@ 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 {
- return properlyDominatesImpl(a, b, enclosingOpOk);
- }
+ bool enclosingOpOk = true) const;
/// Return true if operation A dominates operation B, i.e. if A and B are the
/// same operation or A properly dominates B.
@@ -187,13 +185,6 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
bool properlyDominates(Block *a, Block *b) const {
return super::properlyDominates(a, b);
}
-
-private:
- // 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 properlyDominatesImpl(Operation *a, Operation *b,
- bool enclosingOpOk) const;
};
/// A class for computing basic postdominance information.
diff --git a/mlir/lib/IR/Dominance.cpp b/mlir/lib/IR/Dominance.cpp
index 31f7e7dbc925ce..bdd0febfa546e0 100644
--- a/mlir/lib/IR/Dominance.cpp
+++ b/mlir/lib/IR/Dominance.cpp
@@ -230,7 +230,7 @@ bool DominanceInfoBase<IsPostDom>::properlyDominates(Block *a, Block *b) const {
if (regionA != b->getParent()) {
b = regionA ? regionA->findAncestorBlockInRegion(*b) : nullptr;
// If we could not find a valid block b then it is a not a dominator.
- if (b == nullptr)
+ if (!b)
return false;
// Check to see if the ancestor of `b` is the same block as `a`. A properly
@@ -266,8 +266,8 @@ template class detail::DominanceInfoBase</*IsPostDom=*/false>;
/// 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::properlyDominatesImpl(Operation *a, Operation *b,
- bool enclosingOpOk) const {
+bool DominanceInfo::properlyDominates(Operation *a, Operation *b,
+ bool enclosingOpOk) const {
Block *aBlock = a->getBlock(), *bBlock = b->getBlock();
assert(aBlock && bBlock && "operations must be in a block");
@@ -319,7 +319,7 @@ bool DominanceInfo::properlyDominates(Value a, Operation *b) const {
// `a` properlyDominates `b` if the operation defining `a` properlyDominates
// `b`, but `a` does not itself enclose `b` in one of its regions.
- return properlyDominatesImpl(a.getDefiningOp(), b, /*enclosingOpOk=*/false);
+ return properlyDominates(a.getDefiningOp(), b, /*enclosingOpOk=*/false);
}
//===----------------------------------------------------------------------===//
|
68c43c6
to
2052c65
Compare
Base automatically changed from
users/matthias-springer/properly_dominate
to
main
November 10, 2024 06:20
58fbba4
to
f997040
Compare
f997040
to
8bc07a5
Compare
River707
approved these changes
Nov 12, 2024
chelini
approved these changes
Nov 12, 2024
matthias-springer
added a commit
that referenced
this pull request
Nov 12, 2024
…ation (#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 #115430.
Groverkss
pushed a commit
to iree-org/llvm-project
that referenced
this pull request
Nov 15, 2024
Remove `properlyDominatesImpl` and implement the functionality in `properlyDominates` directly.
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.
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.
Remove
properlyDominatesImpl
and implement the functionality inproperlyDominates
directly.