[mlir][scf] Loop peeling makes a dynamic zero-trip loop execute, because the bail-out guards are constant-only (both directions)
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
## Summary
`-scf-for-loop-peeling` and `-scf-for-loop-peeling=peel-front=true` both split an
`scf.for` into a main loop and a partial iteration. Each has a guard that bails
out when peeling is unnecessary, but **both guards require every bound to be a
bare constant**. When any bound is dynamic the guard is skipped and the partial
iteration is emitted unconditionally — including for loops whose trip count is
zero.
`scf.for`'s contract defines the trip count as `max(0, ceil((ub - lb) / step))`,
so a loop with `lb >= ub` must not execute at all. After peeling it does.
Front peeling, `LoopSpecialization.cpp:236`:
```c++
// Peeling is not needed if there is one or less iteration.
if (lbInt && ubInt && stepInt && ceil(float(*ubInt - *lbInt) / *stepInt) <= 1)
return failure();
```
Back peeling, `LoopSpecialization.cpp:137`:
```c++
// No specialization necessary if step already divides upper bound evenly.
// Fast path: lb, ub and step are constants.
if (lbInt && ubInt && stepInt && (*ubInt - *lbInt) % *stepInt == 0)
return failure();
```
`lbInt`, `ubInt` and `stepInt` all come from `getConstantIntValue`, so an
ordinary dynamic bound makes each condition false and execution falls through to
the peeling transform. The resulting IR is verifier-clean, so this is a silent
wrong-result miscompilation.
Both reproduce at `60b5f9a06722`.
## Reproducer
With `lb = 1, ub = 0, step = 2` the trip count is `max(0, ceil(-1/2)) = 0`, so
the body must never run and the result must be `0`. The bounds are function
arguments, which is what defeats the guards.
```mlir
func.func @count(%lb: index, %ub: index) -> i32 {
%c0 = arith.constant 0 : index
%c2 = arith.constant 2 : index
%one = arith.constant 1 : i32
%zero = arith.constant 0 : i32
%m = memref.alloca() : memref<1xi32>
memref.store %zero, %m[%c0] : memref<1xi32>
scf.for %i = %lb to %ub step %c2 {
%o = memref.load %m[%c0] : memref<1xi32>
%n = arith.addi %o, %one : i32
memref.store %n, %m[%c0] : memref<1xi32>
}
%r = memref.load %m[%c0] : memref<1xi32>
return %r : i32
}
func.func @main() {
%lb = arith.constant 1 : index
%ub = arith.constant 0 : index
%r = func.call @count(%lb, %ub) : (index, index) -> i32
vector.print %r : i32
return
}
```
```
PIPE="-lower-affine -convert-scf-to-cf -convert-cf-to-llvm -convert-vector-to-llvm \
-finalize-memref-to-llvm -convert-func-to-llvm -convert-arith-to-llvm \
-reconcile-unrealized-casts"
mlir-opt $PIPE repro.mlir | mlir-runner -e main -entry-point-result=void \
--shared-libs=$LIB/libmlir_runner_utils.so
# => 0 (correct: the loop is empty)
mlir-opt -scf-for-loop-peeling=peel-front=true $PIPE repro.mlir | mlir-runner ...
# => 1 (wrong: the peeled first iteration ran)
mlir-opt -scf-for-loop-peeling $PIPE repro.mlir | mlir-runner ...
# => 1 (wrong: the partial iteration ran)
```
The same defect is reachable through `transform.loop.peel`.
## Why the guards do not catch it
Both guards are written as fast paths for the fully-constant case and have no
fallback. Nothing establishes `ub > lb` before the partial iteration is emitted,
so for any runtime state with `lb >= ub` the peeled iteration executes a body the
source never executes.
## Environment
* llvm-project at `60b5f9a06722`
* Release build with assertions, Linux x86-64
Contributor guide
Research direction
Start in LoopSpecialization.cpp at the front-peeling guard around line 236 and back-peeling guard around line 137; inspect -scf-for-loop-peeling and transform.loop.peel using the supplied MLIR reproducer. Run the shown mlir-opt/mlir-runner pipeline for dynamic bounds including lb=1, ub=0, step=2; done means both peeling modes preserve the empty-loop result (0) rather than execute the partial iteration.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100