[PATCH] D78012: [MLIR] Fix dominance info method - properlyDominates

[Uday Bondhugula via Phabricator via llvm-commits]

bondhugula created this revision.
bondhugula added reviewers: lattner, mehdi_amini.
Herald added subscribers: llvm-commits, frgossen, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar, rriddle.
Herald added a project: LLVM.
bondhugula updated this revision to Diff 256955.
bondhugula added a comment.

Update comments.


Fix properlyDominates for the case where an operation could dominate
another one appearing before it in the same block. The only paths to the
latter could be from the former.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78012

Files:
  mlir/lib/Analysis/Dominance.cpp
  mlir/test/IR/dominance.mlir


Index: mlir/test/IR/dominance.mlir
===================================================================
--- /dev/null
+++ mlir/test/IR/dominance.mlir
@@ -0,0 +1,38 @@
+// RUN: mlir-opt %s | FileCheck %s
+
+// Verify dominance related functions in "cyclic" cases.
+// CHECK-LABEL: func @dominance
+func @dominance(%n: i32, %p : i1) -> i32 {
+  %x = addi %n, %n : i32
+  return %x : i32
+
+  // Self loop.
+^bar:
+  %y = addi %z, %x : i32
+  // muli does dominate addi.
+  %z = muli %y, %y : i32
+  br ^bar
+
+  // abc and xyz cycle.
+^abc:
+  %a = addi %b, %x : i32
+  // muli does dominate addi.
+  %b = muli %a, %a : i32
+  br ^xyz
+
+^xyz:
+  br ^abc
+
+  // c1, c2, c3 are an SCC.
+^c1:
+  %u = addi %v, %x : i32
+  // muli does dominate addi.
+  %v = muli %u, %u : i32
+  cond_br %p, ^c2, ^c3
+
+^c2:
+  br ^c1
+
+^c3:
+  br ^c1
+}
Index: mlir/lib/Analysis/Dominance.cpp
===================================================================
--- mlir/lib/Analysis/Dominance.cpp
+++ mlir/lib/Analysis/Dominance.cpp
@@ -183,17 +183,29 @@
 // DominanceInfo
 //===----------------------------------------------------------------------===//
 
-/// Return true if operation A properly dominates operation B.
+/// Returns true if operation A properly dominates operation B.
 bool DominanceInfo::properlyDominates(Operation *a, Operation *b) {
   auto *aBlock = a->getBlock(), *bBlock = b->getBlock();
 
-  // If a or b are not within a block, then a does not dominate b.
+  // If a or b are not within a block, then a does not properly dominate b.
   if (!aBlock || !bBlock)
     return false;
 
   // If the blocks are the same, then check if b is before a in the block.
-  if (aBlock == bBlock)
-    return a->isBeforeInBlock(b);
+  if (aBlock == bBlock) {
+    if (a->isBeforeInBlock(b))
+      return true;
+
+    // Even if b is before a in their block, if aBlock dominates all
+    // predecessors of aBlock, a dominates b.
+
+    // Can't be equal for proper dominance.
+    if (a == b || aBlock->hasNoPredecessors())
+      return false;
+
+    return llvm::all_of(aBlock->getPredecessors(),
+                        [&](Block *p) { return dominates(aBlock, p); });
+  }
 
   // Traverse up b's hierarchy to check if b's block is contained in a's.
   if (auto *bAncestor = aBlock->findAncestorOpInBlock(*b)) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78012.256955.patch
Type: text/x-patch
Size: 2311 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200413/feef3891/attachment.bin>