Skip to content

Commit

Permalink
[mlir][IR] DominanceInfo: Fix inconsistency in proper block/op domi…
Browse files Browse the repository at this point in the history
…nance (#115413)

An operation is considered to properly dominate itself in a graph
region. That's because there is no concept of "dominance" in a graph
region. (`dominates` returns "true" for all pairs of ops in the same
block. It makes sense to do the same for `properlyDominates`.)

Previously, a block was *not* considered to dominate itself in a graph
region. This commit fixes this asymmetry between ops and blocks: both
are now properly dominating themselves in a graph region.
  • Loading branch information
matthias-springer authored Nov 10, 2024
1 parent f344367 commit e4c1419
Show file tree
Hide file tree
Showing 4 changed files with 645 additions and 238 deletions.
15 changes: 10 additions & 5 deletions mlir/include/mlir/IR/Dominance.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
/// are in the same block and A properly dominates B within the block, or if
/// the block that contains A properly dominates the block that contains B. In
/// an SSACFG region, Operation A dominates Operation B in the same block if A
/// preceeds B. In a Graph region, all operations in a block dominate all
/// other operations in the same block.
/// preceeds B. In a Graph region, all operations in a block properly dominate
/// all operations in the same block.
///
/// The `enclosingOpOk` flag says whether we should return true if the B op
/// is enclosed by a region on A.
Expand Down Expand Up @@ -176,9 +176,14 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
/// Return true if the specified block A properly dominates block B, i.e.: if
/// block A contains block B, or if the region which contains block A also
/// contains block B or some parent of block B and block A dominates that
/// block in that kind of region. In an SSACFG region, block A dominates
/// block B if all control flow paths from the entry block to block B flow
/// through block A. In a Graph region, all blocks dominate all other blocks.
/// block in that kind of region.
///
/// In an SSACFG region, block A dominates block B if all control flow paths
/// from the entry block to block B flow through block A.
///
/// 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::properlyDominates(a, b);
}
Expand Down
10 changes: 6 additions & 4 deletions mlir/lib/IR/Dominance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ DominanceInfoBase<IsPostDom>::~DominanceInfoBase() {
delete entry.second.getPointer();
}

template <bool IsPostDom> void DominanceInfoBase<IsPostDom>::invalidate() {
template <bool IsPostDom>
void DominanceInfoBase<IsPostDom>::invalidate() {
for (auto entry : dominanceInfos)
delete entry.second.getPointer();
dominanceInfos.clear();
Expand Down Expand Up @@ -217,9 +218,10 @@ template <bool IsPostDom>
bool DominanceInfoBase<IsPostDom>::properlyDominates(Block *a, Block *b) const {
assert(a && b && "null blocks not allowed");

// A block dominates itself but does not properly dominate itself.
// A block dominates, but does not properly dominate, itself unless this
// is a graph region.
if (a == b)
return false;
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
Expand Down Expand Up @@ -269,7 +271,7 @@ bool DominanceInfo::properlyDominatesImpl(Operation *a, Operation *b,
Block *aBlock = a->getBlock(), *bBlock = b->getBlock();
assert(aBlock && bBlock && "operations must be in a block");

// An instruction dominates, but does not properlyDominate, itself unless this
// An operation dominates, but does not properly dominate, itself unless this
// is a graph region.
if (a == b)
return !hasSSADominance(aBlock);
Expand Down
Loading

0 comments on commit e4c1419

Please sign in to comment.