Commit cbe7c49
[mlir][memref] Fix runtime verification for memref.subview when size dimension value is 0 (llvm#164897)
Previously, the runtime verification pass would insert assertion
statements with conditions that always evaluate to false for
semantically valid `memref.subview` operations where one of the
dimensions had a size of 0.
The `memref.subview` runtime verification logic was unconditionally
generating checks for the position of the last element (`offset + (size
- 1) * stride`). When `size` is 0, this causes the assertion condition
to always be false, leading to runtime failures even though the
operation is semantically valid.
This patch fixes the issue by making the `lastPos` check conditional.
The offset is always verified, but the endpoint check is only performed
when `size > 0` to avoid generating spurious assert statements.
This issue was discovered through a LiteRT model, where a dynamic shape
calculation resulted in a zero-sized dimension being passed to
`memref.subview`. The following is a simplified IR snippet from the
model. After running the runtime verification pass, an assertion that
always fails is generated because the SSA value `%5` becomes 0.
```mlir
module {
memref.global "private" constant @__constant_2xi32 : memref<2xi32> = dense<-1> {alignment = 64 : i64}
memref.global "private" constant @__constant_1xi32 : memref<1xi32> = dense<0> {alignment = 64 : i64}
func.func @simpleRepro(%arg0: memref<10x4x1xf32, strided<[?, ?, ?], offset: ?>>) -> memref<?x?x?xf32, strided<[?, ?, ?], offset: ?>> {
%c2 = arith.constant 2 : index
%c4 = arith.constant 4 : index
%c1 = arith.constant 1 : index
%c10 = arith.constant 10 : index
%c0 = arith.constant 0 : index
%c-1 = arith.constant -1 : index
%0 = memref.get_global @__constant_1xi32 : memref<1xi32>
%1 = memref.get_global @__constant_2xi32 : memref<2xi32>
%alloca = memref.alloca() {alignment = 64 : i64} : memref<3xi32>
%subview = memref.subview %alloca[0] [1] [1] : memref<3xi32> to memref<1xi32, strided<[1]>>
memref.copy %0, %subview : memref<1xi32> to memref<1xi32, strided<[1]>>
%subview_0 = memref.subview %alloca[1] [2] [1] : memref<3xi32> to memref<2xi32, strided<[1], offset: 1>>
memref.copy %1, %subview_0 : memref<2xi32> to memref<2xi32, strided<[1], offset: 1>>
%2 = memref.load %alloca[%c0] : memref<3xi32>
%3 = index.casts %2 : i32 to index
%4 = arith.cmpi eq, %3, %c-1 : index
%5 = arith.select %4, %c10, %3 : index
%6 = memref.load %alloca[%c1] : memref<3xi32>
%7 = index.casts %6 : i32 to index
%8 = arith.cmpi eq, %7, %c-1 : index
%9 = arith.select %8, %c4, %7 : index
%10 = memref.load %alloca[%c2] : memref<3xi32>
%11 = index.casts %10 : i32 to index
%12 = arith.cmpi eq, %11, %c-1 : index
%13 = arith.select %12, %c1, %11 : index
%subview_1 = memref.subview %arg0[0, 0, 0] [%5, %9, %13] [1, 1, 1] : memref<10x4x1xf32, strided<[?, ?, ?], offset: ?>> to memref<?x?x?xf32, strided<[?, ?, ?], offset: ?>>
return %subview_1 : memref<?x?x?xf32, strided<[?, ?, ?], offset: ?>>
}
}
```
P.S. This is a similar issue to the one fixed for `tensor.extract_slice`
in llvm#164878
---------
Co-authored-by: Hanumanth Hanumantharayappa <hhanuman@ah-hhanuman-l.dhcp.mathworks.com>1 parent a6788b5 commit cbe7c49
File tree
2 files changed
+48
-2
lines changed- mlir
- lib/Dialect/MemRef/Transforms
- test/Integration/Dialect/MemRef
2 files changed
+48
-2
lines changedLines changed: 27 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
15 | 15 | | |
16 | 16 | | |
17 | 17 | | |
| 18 | + | |
18 | 19 | | |
19 | 20 | | |
20 | 21 | | |
| |||
273 | 274 | | |
274 | 275 | | |
275 | 276 | | |
276 | | - | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
277 | 280 | | |
278 | 281 | | |
279 | 282 | | |
| |||
290 | 293 | | |
291 | 294 | | |
292 | 295 | | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
293 | 306 | | |
294 | 307 | | |
295 | 308 | | |
| |||
298 | 311 | | |
299 | 312 | | |
300 | 313 | | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
301 | 326 | | |
302 | | - | |
| 327 | + | |
303 | 328 | | |
304 | 329 | | |
305 | 330 | | |
| |||
Lines changed: 21 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
11 | 12 | | |
12 | 13 | | |
13 | 14 | | |
| 15 | + | |
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
| |||
38 | 40 | | |
39 | 41 | | |
40 | 42 | | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
41 | 54 | | |
42 | 55 | | |
43 | 56 | | |
| |||
105 | 118 | | |
106 | 119 | | |
107 | 120 | | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
108 | 129 | | |
109 | 130 | | |
110 | 131 | | |
0 commit comments