[Mlir-commits] [mlir] 9de4ee3 - [MLIR] Allow unreachable blocks to violate dominance property.

[Stephen Neuendorffer]

Author: Stephen Neuendorffer
Date: 2020-05-15T10:31:57-07:00
New Revision: 9de4ee3815db2b49960e931bcc3c0cc6a28ad0de

URL: https://github.com/llvm/llvm-project/commit/9de4ee3815db2b49960e931bcc3c0cc6a28ad0de
DIFF: https://github.com/llvm/llvm-project/commit/9de4ee3815db2b49960e931bcc3c0cc6a28ad0de.diff

LOG: [MLIR] Allow unreachable blocks to violate dominance property.

It is possible for optimizations to create SSA code which violates
the dominance property in unreachable blocks.  Equivalently, dominance
computed using normal mechanisms is undefined in unreachable blocks.

See discussion here: https://llvm.discourse.group/t/rfc-allowing-dialects-to-relax-the-ssa-dominance-condition/833/51

This patch only checks the dominance condition inside blocks which are
reachable from the the entry block of their region.  Note that the
dominance conditions of regions contained in an unreachable block are
still checked.

Differential Revision: https://reviews.llvm.org/D79922

Added: 
    

Modified: 
    mlir/include/mlir/IR/Dominance.h
    mlir/lib/IR/Dominance.cpp
    mlir/lib/IR/Verifier.cpp
    mlir/test/IR/invalid.mlir
    mlir/test/IR/parser.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/Dominance.h b/mlir/include/mlir/IR/Dominance.h
index 27ea3c6948e7..97ec99007708 100644
--- a/mlir/include/mlir/IR/Dominance.h
+++ b/mlir/include/mlir/IR/Dominance.h
@@ -54,6 +54,10 @@ template <bool IsPostDom> class DominanceInfoBase {
   /// Return true if the specified block A properly dominates block B.
   bool properlyDominates(Block *a, Block *b) const;
 
+  /// Return true if the specified block is reachable from the entry
+  /// block of its region.
+  bool isReachableFromEntry(Block *a) const;
+
   /// A mapping of regions to their base dominator tree.
   DenseMap<Region *, std::unique_ptr<base>> dominanceInfos;
 };
@@ -64,6 +68,12 @@ class DominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/false> {
 public:
   using super::super;
 
+  /// Return true if the specified block is reachable from the entry
+  /// block of its region.
+  bool isReachableFromEntry(Block *a) const {
+    return super::isReachableFromEntry(a);
+  }
+
   /// Return true if operation A properly dominates operation B.
   bool properlyDominates(Operation *a, Operation *b) const;
 
@@ -99,6 +109,12 @@ class PostDominanceInfo : public detail::DominanceInfoBase</*IsPostDom=*/true> {
 public:
   using super::super;
 
+  /// Return true if the specified block is reachable from the entry
+  /// block of its region.
+  bool isReachableFromEntry(Block *a) const {
+    return super::isReachableFromEntry(a);
+  }
+
   /// Return true if operation A properly postdominates operation B.
   bool properlyPostDominates(Operation *a, Operation *b);
 

diff  --git a/mlir/lib/IR/Dominance.cpp b/mlir/lib/IR/Dominance.cpp
index 86313adc4c0d..f7a6ac35eaee 100644
--- a/mlir/lib/IR/Dominance.cpp
+++ b/mlir/lib/IR/Dominance.cpp
@@ -176,6 +176,17 @@ bool DominanceInfoBase<IsPostDom>::properlyDominates(Block *a, Block *b) const {
   return baseInfoIt->second->properlyDominates(a, b);
 }
 
+/// Return true if the specified block is reachable from the entry block of its
+/// region.
+template <bool IsPostDom>
+bool DominanceInfoBase<IsPostDom>::isReachableFromEntry(Block *a) const {
+  auto *regionA = a->getParent();
+  auto baseInfoIt = dominanceInfos.find(regionA);
+  if (baseInfoIt == dominanceInfos.end())
+    return true;
+  return baseInfoIt->second->isReachableFromEntry(a);
+}
+
 template class mlir::detail::DominanceInfoBase</*IsPostDom=*/true>;
 template class mlir::detail::DominanceInfoBase</*IsPostDom=*/false>;
 

diff  --git a/mlir/lib/IR/Verifier.cpp b/mlir/lib/IR/Verifier.cpp
index 763758193c5d..20d3a9587463 100644
--- a/mlir/lib/IR/Verifier.cpp
+++ b/mlir/lib/IR/Verifier.cpp
@@ -224,9 +224,19 @@ LogicalResult OperationVerifier::verifyOperation(Operation &op) {
 LogicalResult OperationVerifier::verifyDominance(Region &region) {
   // Verify the dominance of each of the held operations.
   for (auto &block : region)
-    for (auto &op : block)
-      if (failed(verifyDominance(op)))
-        return failure();
+    // Dominance is only reachable inside reachable blocks.
+    if (domInfo->isReachableFromEntry(&block))
+      for (auto &op : block) {
+        if (failed(verifyDominance(op)))
+          return failure();
+      }
+    else
+      // Verify the dominance of each of the nested blocks within this
+      // operation, even if the operation itself is not reachable.
+      for (auto &op : block)
+        for (auto &region : op.getRegions())
+          if (failed(verifyDominance(region)))
+            return failure();
   return success();
 }
 

diff  --git a/mlir/test/IR/invalid.mlir b/mlir/test/IR/invalid.mlir
index 47fd0b7a371f..d51d7d8839d3 100644
--- a/mlir/test/IR/invalid.mlir
+++ b/mlir/test/IR/invalid.mlir
@@ -1527,3 +1527,22 @@ func @forward_reference_type_check() -> (i8) {
   %1 = "bar"() : () -> (f32)
   br ^bb1
 }
+
+// -----
+
+func @dominance_error_in_unreachable_op() -> i1 {
+  %c = constant 0 : i1
+  return %c : i1
+^bb0:
+  "dummy" () ({  // unreachable
+    ^bb1:
+// expected-error @+1 {{operand #0 does not dominate this use}}
+      %2:3 = "bar"(%1) : (i64) -> (i1,i1,i1)
+      br ^bb4
+    ^bb2:
+      br ^bb2
+    ^bb4:
+      %1 = "foo"() : ()->i64   // expected-note {{operand defined here}}
+  }) : () -> ()
+  return %c : i1
+}

diff  --git a/mlir/test/IR/parser.mlir b/mlir/test/IR/parser.mlir
index 34b1da4282f1..34acb5eec6e9 100644
--- a/mlir/test/IR/parser.mlir
+++ b/mlir/test/IR/parser.mlir
@@ -1243,3 +1243,17 @@ func @pretty_names() {
   // CHECK: return
   return
 }
+
+func @unreachable_dominance_violation_ok() -> i1 {
+  %c = constant 0 : i1       // CHECK: [[VAL:%.*]] = constant 0 : i1
+  return %c : i1    // CHECK:   return [[VAL]] : i1
+^bb1:         // CHECK: ^bb1:   // no predecessors
+  // %1 is not dominated by it's definition, but block is not reachable.
+  %2:3 = "bar"(%1) : (i64) -> (i1,i1,i1) // CHECK: [[VAL2:%.*]]:3 = "bar"([[VAL3:%.*]]) : (i64) -> (i1, i1, i1)
+  br ^bb4     // CHECK:   br ^bb3
+^bb2:         // CHECK: ^bb2:   // pred: ^bb2
+  br ^bb2     // CHECK:   br ^bb2
+^bb4:         // CHECK: ^bb3:   // pred: ^bb1
+  %1 = "foo"() : ()->i64 // CHECK: [[VAL3]] = "foo"() : () -> i64
+  return %2#1 : i1 // CHECK: return [[VAL2]]#1 : i1
+}            // CHECK: }