llvm / llvm/llvm-project

[mlir][scf] Loop peeling makes a dynamic zero-trip loop execute, because the bail-out guards are constant-only (both directions)

Open
#223,232 2 comments 0 reactions 0 assignees View on GitHub
mlir:scf
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.