Fix nested dominance relationship between parent results and child op…

…erations.

This modifies DominanceInfo::properlyDominates(Value *value, Operation *op) to return false if the value is defined by a parent operation of 'op'. This prevents using values defined by the parent operation from within any child regions.

PiperOrigin-RevId: 269934920

mlir/include/mlir/IR/Operation.h

@@ -139,6 +139,17 @@ class Operation final
     return OpTy();
   }
 
+  /// Return true if this operation is a proper ancestor of the `other`
+  /// operation.
+  bool isProperAncestor(Operation *other);
+
+  /// Return true if this operation is an ancestor of the `other` operation. An
+  /// operation is considered as its own ancestor, use `isProperAncestor` to
+  /// avoid this.
+  bool isAncestor(Operation *other) {
+    return this == other || isProperAncestor(other);
+  }
+
   /// Replace any uses of 'from' with 'to' within this operation.
   void replaceUsesOfWith(Value *from, Value *to);
 

mlir/lib/Analysis/Dominance.cpp

@@ -128,8 +128,14 @@ bool DominanceInfo::properlyDominates(Operation *a, Operation *b) {
 
 /// Return true if value A properly dominates operation B.
 bool DominanceInfo::properlyDominates(Value *a, Operation *b) {
-  if (auto *aInst = a->getDefiningOp())
+  if (auto *aInst = a->getDefiningOp()) {
+    // The values defined by an operation do *not* dominate any nested
+    // operations.
+    if (aInst->getParentRegion() != b->getParentRegion() &&
+        aInst->isAncestor(b))
+      return false;
     return properlyDominates(aInst, b);
+  }
 
   // block arguments properly dominate all operations in their own block, so
   // we use a dominates check here, not a properlyDominates check.

mlir/lib/IR/Operation.cpp

@@ -280,6 +280,15 @@ Operation *Operation::getParentOp() {
   return block ? block->getParentOp() : nullptr;
 }
 
+/// Return true if this operation is a proper ancestor of the `other`
+/// operation.
+bool Operation::isProperAncestor(Operation *other) {
+  while ((other = other->getParentOp()))
+    if (this == other)
+      return true;
+  return false;
+}
+
 /// Replace any uses of 'from' with 'to' within this operation.
 void Operation::replaceUsesOfWith(Value *from, Value *to) {
   if (from == to)

mlir/test/IR/invalid.mlir

@@ -1064,6 +1064,18 @@ func @invalid_region_dominance() {
 
 // -----
 
+func @invalid_region_dominance() {
+  // expected-note @+1 {{operand defined here}}
+  %def = "foo.region_with_def"() ({
+    // expected-error @+1 {{operand #0 does not dominate this use}}
+    "foo.use" (%def) : (i32) -> ()
+    "foo.yield" () : () -> ()
+  }) : () -> (i32)
+  return
+}
+
+// -----
+
 func @hexadecimal_bf16() {
   // expected-error @+1 {{integer literal not valid for specified type}}
   "foo"() {value = 0xffff : bf16} : () -> ()