[MLIR][Affine] `affine-loop-invariant-code-motion` hoists a region op without checking what its region captures
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
`isOpLoopInvariant` recurses into the regions of exactly three operations — `affine.if`, `affine.for`, `affine.parallel` — and treats every other region-carrying operation as if its regions were empty, examining only `op.getOperands()`. An `scf.for` whose bounds, step and initial `iter_args` are all loop-invariant therefore looks hoistable even when its body reads a value defined inside the loop, and the hoisted operation is placed before the `affine.for` with its region still referring to a value that stayed behind.
## Reproducer
### small.mlir
```mlir
func.func @f(%m: memref<4xi64>, %init: i64, %outside: i64) -> i64 {
%c0 = arith.constant 0 : i32
%c4 = arith.constant 4 : i32
%c2 = arith.constant 2 : i32
%one = arith.constant 1 : i64
%r = affine.for %i = 0 to 4 iter_args(%acc = %init) -> (i64) {
%v = affine.load %m[%i] : memref<4xi64>
%s = scf.for %j = %c0 to %c4 step %c2 iter_args(%a = %outside) -> (i64) : i32 {
%o = arith.ori %v, %one : i64
%n = arith.addi %a, %o : i64
scf.yield %n : i64
}
%acc2 = arith.addi %acc, %s : i64
affine.yield %acc2 : i64
}
return %r : i64
}
```
### To reproduce
```
$ mlir-opt small.mlir -o /dev/null
(input parses and verifies)
$ mlir-opt small.mlir --affine-loop-invariant-code-motion -o /dev/null
small.mlir:12:12: error: operand #0 does not dominate this use
%o = arith.ori %v, %one : i64
^
small.mlir:8:10: note: operand defined here (op is neither in a parent nor in a child region)
%v = affine.load %m[%i] : memref<4xi64>
```
With the verifier switched off the transformation is plain — the `scf.for` now sits *above* the `affine.for` and reads `%6`, which is defined below it, inside the loop body:
```bash
$ mlir-opt small.mlir --affine-loop-invariant-code-motion --verify-each=false -o -
```
```mlir
%4 = "scf.for"(%0, %1, %2, %arg2) ({
^bb0(%arg5: i32, %arg6: i64):
%8 = "arith.ori"(%6, %3) : (i64, i64) -> i64 // %6 is defined below, in the loop
%9 = "arith.addi"(%arg6, %8) : (i64, i64) -> i64
"scf.yield"(%9) : (i64) -> ()
}) : (i32, i32, i32, i64) -> i64
%5 = "affine.for"(%arg1) ({
^bb0(%arg3: index, %arg4: i64):
%6 = "affine.load"(%arg0, %arg3) : (memref<4xi64>, index) -> i64
%7 = "arith.addi"(%arg4, %4) : (i64, i64) -> i64
"affine.yield"(%7) : (i64) -> ()
}) : (i64) -> i64
```
### Actual Output:
```
error: operand #0 does not dominate this use
```
### Expected Output:
```
(the pass either hoists legally or declines; the module still verifies)
```
Contributor guide
Research direction
Start by reproducing the failure with small.mlir using mlir-opt --affine-loop-invariant-code-motion, then inspect isOpLoopInvariant and how the pass handles region-carrying operations. Add coverage for the nested scf.for case and verify that the transformed module remains valid, either by legal hoisting or by declining to hoist it.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100