Skip to content

Commit e8c5e29

Browse files
[mlir][IR] Fix inconsistency in block/op dominance
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.) Previously, a block was *not* considered to dominate itself in a graph region. This commit fixes thise asymmetry between ops and blocks: both are now properly dominating themselves in a graph region.
1 parent 36d757f commit e8c5e29

File tree

2 files changed

+16
-9
lines changed

2 files changed

+16
-9
lines changed

mlir/include/mlir/IR/Dominance.h

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,8 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
141141
/// are in the same block and A properly dominates B within the block, or if
142142
/// the block that contains A properly dominates the block that contains B. In
143143
/// an SSACFG region, Operation A dominates Operation B in the same block if A
144-
/// preceeds B. In a Graph region, all operations in a block dominate all
145-
/// other operations in the same block.
144+
/// preceeds B. In a Graph region, all operations in a block properly dominate
145+
/// all operations in the same block.
146146
///
147147
/// The `enclosingOpOk` flag says whether we should return true if the B op
148148
/// is enclosed by a region on A.
@@ -176,9 +176,14 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
176176
/// Return true if the specified block A properly dominates block B, i.e.: if
177177
/// block A contains block B, or if the region which contains block A also
178178
/// contains block B or some parent of block B and block A dominates that
179-
/// block in that kind of region. In an SSACFG region, block A dominates
180-
/// block B if all control flow paths from the entry block to block B flow
181-
/// through block A. In a Graph region, all blocks dominate all other blocks.
179+
/// block in that kind of region.
180+
///
181+
/// In an SSACFG region, block A dominates block B if all control flow paths
182+
/// from the entry block to block B flow through block A.
183+
///
184+
/// Graph regions have only a single block. To be consistent with "proper
185+
/// dominance" of ops, the single block is considered to properly dominate
186+
/// itself in a graph region.
182187
bool properlyDominates(Block *a, Block *b) const {
183188
return super::properlyDominates(a, b);
184189
}

mlir/lib/IR/Dominance.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ DominanceInfoBase<IsPostDom>::~DominanceInfoBase() {
3434
delete entry.second.getPointer();
3535
}
3636

37-
template <bool IsPostDom> void DominanceInfoBase<IsPostDom>::invalidate() {
37+
template <bool IsPostDom>
38+
void DominanceInfoBase<IsPostDom>::invalidate() {
3839
for (auto entry : dominanceInfos)
3940
delete entry.second.getPointer();
4041
dominanceInfos.clear();
@@ -217,9 +218,10 @@ template <bool IsPostDom>
217218
bool DominanceInfoBase<IsPostDom>::properlyDominates(Block *a, Block *b) const {
218219
assert(a && b && "null blocks not allowed");
219220

220-
// A block dominates itself but does not properly dominate itself.
221+
// A block dominates, but does not properly dominate, itself unless this
222+
// is a graph region.
221223
if (a == b)
222-
return false;
224+
return !hasSSADominance(a);
223225

224226
// If both blocks are not in the same region, `a` properly dominates `b` if
225227
// `b` is defined in an operation region that (recursively) ends up being
@@ -269,7 +271,7 @@ bool DominanceInfo::properlyDominatesImpl(Operation *a, Operation *b,
269271
Block *aBlock = a->getBlock(), *bBlock = b->getBlock();
270272
assert(aBlock && bBlock && "operations must be in a block");
271273

272-
// An instruction dominates, but does not properlyDominate, itself unless this
274+
// An operation dominates, but does not properly dominate, itself unless this
273275
// is a graph region.
274276
if (a == b)
275277
return !hasSSADominance(aBlock);

0 commit comments

Comments
 (0)