[Mlir-commits] [mlir] [MLIR] Fix control-flow sinking through nested regions (PR #217168)

[llvmlistbot at llvm.org]

================
@@ -80,8 +80,11 @@ bool Sinker::allUsersDominatedBy(Operation *op, Region *region) {
          "expected op to be defined outside the region");
   return llvm::all_of(op->getUsers(), [&](Operation *user) {
     // The user is dominated by the region if its containing block is dominated
-    // by the region's entry block.
-    return domInfo.dominates(&region->front(), user->getBlock());
+    // by the region's entry block. Additionally, allow users that are in
+    // descendant regions of the region (e.g., nested loops) since those
+    // should still permit sinking into the outer region.
+    return domInfo.dominates(&region->front(), user->getBlock()) ||
+           region->isAncestor(user->getParentRegion());
----------------
MarkVeerasingam wrote:

Thanks for the review, good question. 
The dominance check misses users in nested regions (e.g. an `scf.for` inside an `scf.if`) because those users live in a different parent region whose blocks aren’t dominated by the outer region’s entry. 

If the user’s parent region is a descendant of the target `region->isAncestor(...)`, the user can only execute when the target region executes, so we treat it as dominated for sinking. 

Happy to add in a short comment or a unit test.

https://github.com/llvm/llvm-project/pull/217168