Skip to content

Commit

Permalink
[mlir] Fix missing check on nested op values in LICM
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Nicolas Vasilache committed Jan 5, 2022
1 parent 2ee8154 commit bb2f87a
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
3 changes: 1 addition & 2 deletions mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,14 @@ 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;
}
}
return true;
}


LogicalResult mlir::moveLoopInvariantCode(LoopLikeOpInterface looplike) {
auto &loopBody = looplike.getLoopBody();

Expand Down
30 changes: 30 additions & 0 deletions mlir/test/Transforms/loop-invariant-code-motion.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit bb2f87a

Please sign in to comment.