[Mlir-commits] [mlir] [MLIR][Affine] Reject affine-loop-tile on exact negative dependence distances (PR #196972)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-affine

Author: Soowon Jeong (swjng)

<details>
<summary>Changes</summary>

# [MLIR][Affine] Reject affine-loop-tile on exact negative dependence distances

## Summary

`affine-loop-tile` can currently tile a loop nest even when dependence analysis
finds an exact singleton negative distance, such as `lb == ub == -1`, in one
component. That is still a negative direction component and tiling can change
the execution order inside a tile.

This patch tightens the tiling legality check so any known negative lower bound
rejects tiling, including singleton negative distances.

## Reproducer

```mlir
func.func @<!-- -->tile_exact_negative_dependence(%arg0: memref<5x5xi32>) {
  affine.for %i = 0 to 4 {
    affine.for %j = 1 to 5 {
      %0 = affine.load %arg0[%i + 1, %j - 1] : memref<5x5xi32>
      affine.store %0, %arg0[%i, %j] : memref<5x5xi32>
    }
  }
  return
}
```

Apply:

```sh
mlir-opt repro.mlir -affine-loop-tile="tile-sizes=2,2"
```

Before this patch, the pass tiles the nest. That can reorder the carried
dependence with direction `<+, ->`.

## Why

`isTilingValid` is intended to reject hyper-rectangular tiling when a dependence
has a negative direction component. The previous condition caught strictly
negative intervals, but missed exact singleton negative distances such as
`lb == ub == -1`.

This loop has a carried dependence with direction `<+, ->`. Tiling changes the
intra-tile execution order so a later source iteration writes a cell before an
earlier source iteration reads it.

## Fix

In `isTilingValid`, reject any dependence component whose known lower bound is
negative. This includes both negative intervals and singleton exact negative
distances.

## Test

Added `mlir/test/Dialect/Affine/loop-tiling.mlir` coverage for
`@<!-- -->tile_exact_negative_dependence`, checking that the unsafe loop nest remains
untiled.

Ran:

```sh
ninja -C build mlir-opt FileCheck count not
build/bin/llvm-lit -sv mlir/test/Dialect/Affine/loop-tiling.mlir
```

Result: passed.

## How it was found

Surfaced by an in-progress SMT-solver-based analysis tool for MLIR affine
passes.

## Tool-use disclosure

This change was drafted with OpenAI Codex assistance per LLVM's AI Tool Use
Policy. All code was read, reviewed, and tested locally; the commit should
carry an `Assisted-by: OpenAI Codex` trailer.


---
Full diff: https://github.com/llvm/llvm-project/pull/196972.diff


2 Files Affected:

- (modified) mlir/lib/Dialect/Affine/Analysis/LoopAnalysis.cpp (+2-2) 
- (modified) mlir/test/Dialect/Affine/loop-tiling.mlir (+21) 


``````````diff
diff --git a/mlir/lib/Dialect/Affine/Analysis/LoopAnalysis.cpp b/mlir/lib/Dialect/Affine/Analysis/LoopAnalysis.cpp
index 166d39e88d41e..a515c97f22427 100644
--- a/mlir/lib/Dialect/Affine/Analysis/LoopAnalysis.cpp
+++ b/mlir/lib/Dialect/Affine/Analysis/LoopAnalysis.cpp
@@ -557,10 +557,10 @@ bool mlir::affine::isTilingValid(ArrayRef<AffineForOp> loops) {
                               OpPrintingFlags().skipRegions());
         for (const DependenceComponent &depComp : depComps) {
           if (depComp.lb.has_value() && depComp.ub.has_value() &&
-              *depComp.lb < *depComp.ub && *depComp.ub < 0) {
+              *depComp.lb < 0) {
             LDBG() << "Dependence component lb = " << Twine(*depComp.lb)
                    << " ub = " << Twine(*depComp.ub)
-                   << " is negative  at depth: " << Twine(d)
+                   << " may be negative at depth: " << Twine(d)
                    << " and thus violates the legality rule.";
             return false;
           }
diff --git a/mlir/test/Dialect/Affine/loop-tiling.mlir b/mlir/test/Dialect/Affine/loop-tiling.mlir
index d2aca48e615ae..df1cc05798b14 100644
--- a/mlir/test/Dialect/Affine/loop-tiling.mlir
+++ b/mlir/test/Dialect/Affine/loop-tiling.mlir
@@ -2,6 +2,7 @@
 // RUN: mlir-opt %s -split-input-file -affine-loop-tile="cache-size=512" | FileCheck %s --check-prefix=MODEL
 // RUN: mlir-opt %s -split-input-file -affine-loop-tile="cache-size=0" | FileCheck %s --check-prefix=ZERO-CACHE
 // RUN: mlir-opt %s -split-input-file -affine-loop-tile="tile-size=32 separate" | FileCheck %s --check-prefix=SEPARATE
+// RUN: mlir-opt %s -split-input-file -affine-loop-tile="tile-sizes=2,2" | FileCheck %s --check-prefix=NEG-DEP
 
 // CHECK-DAG: [[$UB:#map[0-9]*]] = affine_map<(d0) -> (d0 + 32)>
 // CHECK-DAG: [[$UB_MIN:#map[0-9]*]] = affine_map<(d0) -> (d0 + 32, 50)>
@@ -327,3 +328,23 @@ func.func @separate_full_tile_1d_max_min(%M : index, %N : index, %P : index, %Q
 // SEPARATE-NEXT:        }
 // SEPARATE-NEXT:      }
 // SEPARATE-NEXT:    }
+
+// -----
+
+// Tiling this nest would violate the dependence with distance (1, -1). The
+// negative component is exact, lb == ub == -1, so the tiling legality check
+// must reject singleton negative distances too.
+// NEG-DEP-LABEL: func.func @tile_exact_negative_dependence
+// NEG-DEP-NEXT:    affine.for %[[I:.*]] = 0 to 4 {
+// NEG-DEP-NEXT:      affine.for %[[J:.*]] = 1 to 5 {
+// NEG-DEP-NEXT:        affine.load %{{.*}}[%[[I]] + 1, %[[J]] - 1]
+// NEG-DEP-NEXT:        affine.store %{{.*}}, %{{.*}}[%[[I]], %[[J]]]
+func.func @tile_exact_negative_dependence(%arg0: memref<5x5xi32>) {
+  affine.for %i = 0 to 4 {
+    affine.for %j = 1 to 5 {
+      %0 = affine.load %arg0[%i + 1, %j - 1] : memref<5x5xi32>
+      affine.store %0, %arg0[%i, %j] : memref<5x5xi32>
+    }
+  }
+  return
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/196972