From 077584f05f79b2e358808afbc4afb5e4aab14dd7 Mon Sep 17 00:00:00 2001 From: Matthias Springer Date: Sat, 9 Nov 2024 07:13:07 +0100 Subject: [PATCH] [mlir][IR][NFC] `DominanceInfo`: Share same impl for block/op dominance 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. --- mlir/include/mlir/IR/Dominance.h | 28 +++---- mlir/lib/IR/Dominance.cpp | 124 ++++++++++++++++++++----------- 2 files changed, 92 insertions(+), 60 deletions(-) diff --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h index 16d17b9c0f3d0..63504cad211a4 100644 --- a/mlir/include/mlir/IR/Dominance.h +++ b/mlir/include/mlir/IR/Dominance.h @@ -113,12 +113,12 @@ class DominanceInfoBase { llvm::PointerIntPair getDominanceInfo(Region *region, bool needsDomTree) 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; + /// Return "true" if block iterator A properly (post)dominates block iterator + /// B. If `enclosingOk` is set, A is considered to (post)dominate B if A + /// encloses B. + bool properlyDominatesImpl(Block *aBlock, Block::iterator aIt, Block *bBlock, + Block::iterator bIt, + bool enclosingOk = true) const; /// A mapping of regions to their base dominator tree and a cached /// "hasSSADominance" bit. This map does not contain dominator trees for @@ -151,9 +151,7 @@ class DominanceInfo : public detail::DominanceInfoBase { /// 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 super::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. @@ -188,9 +186,7 @@ class DominanceInfo : public detail::DominanceInfoBase { /// Graph regions have only a single block. To be consistent with "proper /// 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::properlyDominatesImpl(a, b); - } + bool properlyDominates(Block *a, Block *b) const; }; /// A class for computing basic postdominance information. @@ -200,9 +196,7 @@ class PostDominanceInfo : public detail::DominanceInfoBase { /// Return true if operation A properly postdominates operation B. bool properlyPostDominates(Operation *a, Operation *b, - bool enclosingOpOk = true) const { - return super::properlyDominatesImpl(a, b, enclosingOpOk); - } + bool enclosingOpOk = true) const; /// Return true if operation A postdominates operation B. bool postDominates(Operation *a, Operation *b) const { @@ -210,9 +204,7 @@ class PostDominanceInfo : public detail::DominanceInfoBase { } /// Return true if the specified block A properly postdominates block B. - bool properlyPostDominates(Block *a, Block *b) const { - return super::properlyDominatesImpl(a, b); - } + bool properlyPostDominates(Block *a, Block *b) const; /// Return true if the specified block A postdominates block B. bool postDominates(Block *a, Block *b) const { diff --git a/mlir/lib/IR/Dominance.cpp b/mlir/lib/IR/Dominance.cpp index 406e0f2d62d64..1c54e09d29b9b 100644 --- a/mlir/lib/IR/Dominance.cpp +++ b/mlir/lib/IR/Dominance.cpp @@ -213,61 +213,73 @@ DominanceInfoBase::findNearestCommonDominator(Block *a, return getDomTree(a->getParent()).findNearestCommonDominator(a, b); } -/// Return true if the specified block A properly dominates block B. -template -bool DominanceInfoBase::properlyDominatesImpl(Block *a, - Block *b) const { - assert(a && b && "null blocks not allowed"); +/// Returns the given block iterator if it lies within the region region. +/// Otherwise, otherwise finds the ancestor of the given block iterator that +/// lies within the given region. Returns and "empty" iterator if the latter +/// fails. +/// +/// Note: This is a variant of Region::findAncestorOpInRegion that operates on +/// block iterators instead of ops. +static std::pair +findAncestorIteratorInRegion(Region *r, Block *b, Block::iterator it) { + // Case 1: The iterator lies within the region region. + if (b->getParent() == r) + return std::make_pair(b, it); + + // Otherwise: Find ancestor iterator. Bail if we run out of parent ops. + Operation *parentOp = b->getParentOp(); + if (!parentOp) + return std::make_pair(static_cast(nullptr), Block::iterator()); + Operation *op = r->findAncestorOpInRegion(*parentOp); + if (!op) + return std::make_pair(static_cast(nullptr), Block::iterator()); + return std::make_pair(op->getBlock(), op->getIterator()); +} - // A block dominates, but does not properly dominate, itself unless this - // is a graph region. +/// Given two iterators into the same block, return "true" if `a` is before `b. +/// Note: This is a variant of Operation::isBeforeInBlock that operates on +/// block iterators instead of ops. +static bool isBeforeInBlock(Block *block, Block::iterator a, + Block::iterator b) { if (a == b) - return !hasSSADominance(a); - - // If both blocks are not in the same region, `a` properly dominates `b` if - // `b` is defined in an operation region that (recursively) ends up being - // dominated by `a`. Walk up the list of containers enclosing B. - Region *regionA = a->getParent(); - 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) - return false; - - // Check to see if the ancestor of `b` is the same block as `a`. A properly - // dominates B if it contains an op that contains the B block. - if (a == b) - return true; - } - - // Otherwise, they are two different blocks in the same region, use DomTree. - return getDomTree(regionA).properlyDominates(a, b); + return false; + if (a == block->end()) + return false; + if (b == block->end()) + return true; + return a->isBeforeInBlock(&*b); } template bool DominanceInfoBase::properlyDominatesImpl( - Operation *a, Operation *b, bool enclosingOpOk) const { - Block *aBlock = a->getBlock(), *bBlock = b->getBlock(); - assert(aBlock && bBlock && "operations must be in a block"); + Block *aBlock, Block::iterator aIt, Block *bBlock, Block::iterator bIt, + bool enclosingOk) const { + assert(aBlock && bBlock && "expected non-null blocks"); - // An operation (pos)dominates, but does not properly (pos)dominate, itself - // unless this is a graph region. - if (a == b) + // A block iterator (post)dominates, but does not properly (post)dominate, + // itself unless this is a graph region. + if (aBlock == bBlock && aIt == bIt) return !hasSSADominance(aBlock); - // If these ops are in different regions, then normalize one into the other. + // If the iterators 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 + // Scoot up b's region tree until we find a location 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) + if (!aRegion) { + bBlock = nullptr; + bIt = Block::iterator(); + } else { + std::tie(bBlock, bIt) = + findAncestorIteratorInRegion(aRegion, bBlock, bIt); + } + if (!bBlock) return false; - bBlock = b->getBlock(); - assert(bBlock->getParent() == aRegion); + assert(bBlock->getParent() == aRegion && "expected block in regionA"); // If 'a' encloses 'b', then we consider it to (post)dominate. - if (a == b && enclosingOpOk) + if (aBlock == bBlock && aIt == bIt && enclosingOk) return true; } @@ -279,9 +291,9 @@ bool DominanceInfoBase::properlyDominatesImpl( if (!hasSSADominance(aBlock)) return true; if constexpr (IsPostDom) { - return b->isBeforeInBlock(a); + return isBeforeInBlock(aBlock, bIt, aIt); } else { - return a->isBeforeInBlock(b); + return isBeforeInBlock(aBlock, aIt, bIt); } } @@ -309,6 +321,18 @@ template class detail::DominanceInfoBase; // DominanceInfo //===----------------------------------------------------------------------===// +bool DominanceInfo::properlyDominates(Operation *a, Operation *b, + bool enclosingOpOk) const { + return super::properlyDominatesImpl(a->getBlock(), a->getIterator(), + b->getBlock(), b->getIterator(), + enclosingOpOk); +} + +bool DominanceInfo::properlyDominates(Block *a, Block *b) const { + return super::properlyDominatesImpl(a, a->begin(), b, b->begin(), + /*enclosingOk=*/true); +} + /// 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`. @@ -322,3 +346,19 @@ 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 +//===----------------------------------------------------------------------===// + +bool PostDominanceInfo::properlyPostDominates(Operation *a, Operation *b, + bool enclosingOpOk) const { + return super::properlyDominatesImpl(a->getBlock(), a->getIterator(), + b->getBlock(), b->getIterator(), + enclosingOpOk); +} + +bool PostDominanceInfo::properlyPostDominates(Block *a, Block *b) const { + return super::properlyDominatesImpl(a, a->end(), b, b->end(), + /*enclosingOk=*/true); +}