[Mlir-commits] [mlir] bb2f87a - [mlir] Fix missing check on nested op values in LICM

[Nicolas Vasilache]

Author: Nicolas Vasilache
Date: 2022-01-05T09:31:23-05:00
New Revision: bb2f87af0ac9ad8cc0efcb523e108e715ee9ab93

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

LOG: [mlir] Fix missing check on nested op values in LICM

LICM checks that nested ops depend only on values defined outside
before performing hoisting.
However, it specifically omits to check for terminators which can
lead to SSA violations.
This revision fixes the incorrect behavior.

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

Added: 
    

Modified: 
    mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
    mlir/test/Transforms/loop-invariant-code-motion.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp b/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
index 15462c95357d8..3c8e14aa66bbd 100644
--- a/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
+++ b/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
@@ -66,7 +66,7 @@ static bool canBeHoisted(Operation *op,
   // can be hoisted.
   for (auto &region : op->getRegions()) {
     for (auto &block : region) {
-      for (auto &innerOp : block.without_terminator())
+      for (auto &innerOp : block)
         if (!canBeHoisted(&innerOp, definedOutside))
           return false;
     }
@@ -74,7 +74,6 @@ static bool canBeHoisted(Operation *op,
   return true;
 }
 
-
 LogicalResult mlir::moveLoopInvariantCode(LoopLikeOpInterface looplike) {
   auto &loopBody = looplike.getLoopBody();
 

diff  --git a/mlir/test/Transforms/loop-invariant-code-motion.mlir b/mlir/test/Transforms/loop-invariant-code-motion.mlir
index da9470327705d..488726321da40 100644
--- a/mlir/test/Transforms/loop-invariant-code-motion.mlir
+++ b/mlir/test/Transforms/loop-invariant-code-motion.mlir
@@ -292,3 +292,33 @@ func @parallel_loop_with_invariant() {
   return
 }
 
+// -----
+
+func private @make_val() -> (index)
+
+// CHECK-LABEL: func @nested_uses_inside
+func @nested_uses_inside(%lb: index, %ub: index, %step: index) {
+  %true = arith.constant true
+
+  // Check that ops that contain nested uses to values not defiend outside 
+  // remain in the loop.
+  // CHECK-NEXT: arith.constant
+  // CHECK-NEXT: scf.for
+  // CHECK-NEXT:   call @
+  // CHECK-NEXT:   call @
+  // CHECK-NEXT:   scf.if
+  // CHECK-NEXT:     scf.yield
+  // CHECK-NEXT:   else
+  // CHECK-NEXT:     scf.yield
+  scf.for %i = %lb to %ub step %step {
+    %val = call @make_val() : () -> (index)
+    %val2 = call @make_val() : () -> (index)
+    %r = scf.if %true -> (index) {
+      scf.yield %val: index
+    } else {
+      scf.yield %val2: index
+    }
+  }
+  return 
+}
+