llvm / llvm/llvm-project

[mlir][scf] scf-parallel-loop-tiling guards tail iterations with an unsigned compare, dropping every negative iteration

Open
#223,233 1 comment 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-parallel-loop-tiling` with `no-min-max-bounds=true` guards the tail
iterations of a tiled `scf.parallel` with an **unsigned** comparison
(`mlir/lib/Dialect/SCF/Transforms/ParallelLoopTiling.cpp:153`):

```c++
arith::CmpIOp::create(
b, op.getLoc(), arith::CmpIPredicate::ult, index, outerUpperBound);
```

`scf.parallel` has no unsigned mode: its bounds are signed `index` values and a
negative lower bound is verifier-legal. Under `ult` every negative index is read
as a very large unsigned value, fails the comparison against a non-negative
upper bound, and its iteration is skipped.

Any `scf.parallel` whose range crosses zero silently loses all of its negative
iterations. The generated IR is verifier-clean.

Reproduces at `31a1c03fe67c`.

## Reproducer

`[-3, 4)` with step 1 is seven legal iterations, three of them negative. The
range is deliberately not a multiple of the tile size, so the pass takes the
guarded tail path rather than the static fast path.

```mlir
func.func @main() {
%c0 = arith.constant 0 : index
%c1 = arith.constant 1 : index
%cm3 = arith.constant -3 : index
%c4 = arith.constant 4 : index
%one = arith.constant 1 : i32
%zero = arith.constant 0 : i32
%m = memref.alloca() : memref<1xi32>
memref.store %zero, %m[%c0] : memref<1xi32>
scf.parallel (%i) = (%cm3) to (%c4) step (%c1) {
%o = memref.load %m[%c0] : memref<1xi32>
%n = arith.addi %o, %one : i32
memref.store %n, %m[%c0] : memref<1xi32>
scf.reduce
}
%r = memref.load %m[%c0] : memref<1xi32>
vector.print %r : i32
return
}
```

```
PIPE="-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
# => 7 (correct)

mlir-opt -scf-parallel-loop-tiling="parallel-loop-tile-sizes=3 no-min-max-bounds=true" \
$PIPE repro.mlir | mlir-runner ...
# => 4 (wrong: the three negative iterations were dropped)
```

The guard is visible in the tiled IR:

```mlir
%4 = arith.cmpi ult, %3, %c4 : index
scf.if %5 { ... }
```

## Similarity to #203693

This is a **different defect** in the same guard construction. Issue
[#203693](https://github.com/llvm/llvm-project/issues/203693) concerns the extra
step multiplication; the source here has step 1, so that path is not involved.

Open PR [#203798](https://github.com/llvm/llvm-project/pull/203798) removes the
step multiplication but leaves `CmpIPredicate::ult` unchanged, so it does not fix
this.

## Environment

* llvm-project at `31a1c03fe67c`
* Release build with assertions, Linux x86-64

Contributor guide

Open the contributing guide

Research direction

Read mlir/lib/Dialect/SCF/Transforms/ParallelLoopTiling.cpp around line 153, then run the supplied [-3, 4) reproducer with scf-parallel-loop-tiling and inspect the generated guard. Check existing SCF parallel-loop-tiling tests for the appropriate regression-test location. Done means negative tail iterations are preserved and the reproducer produces seven iterations rather than four.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.