Question about memref.subview verification

MLIR
1

Hi all. Is there semantically any difference between following two programs?:
The difference is offsets operand of memref.subview op ; memref.subview %3[%c0 vs memref.subview %3[0

module  {
  func private @foo(memref<30x30xi32>) -> i32
  func @main() {
    %c0 = arith.constant 0 : index
    %3 = memref.alloca() : memref<1x30x30xi32>
    %4 = memref.subview %3[%c0, 0, 0] [1, 30, 30] [1, 1, 1] : memref<1x30x30xi32> to memref<30x30xi32>
    %7 = call @foo(%4) : (memref<30x30xi32>) -> i32
    return
  }
}
module  {
  func private @foo(memref<30x30xi32>) -> i32
  func @main() {
    %3 = memref.alloca() : memref<1x30x30xi32>
    %4 = memref.subview %3[0, 0, 0] [1, 30, 30] [1, 1, 1] : memref<1x30x30xi32> to memref<30x30xi32>
    %7 = call @foo(%4) : (memref<30x30xi32>) -> i32
    return
  }
}

Only for the first one, mlir-opt foo.mlir gives me a following verification error.
(commit: cac164ff9cb671fae7a00d9a3adf1834ebca8bd9)

foo.mlir:6:10: error: expected result type to be 'memref<1x30x30xi32, affine_map<(d0, d1, d2)[s0] -> (d0 * 900 + s0 + d1 * 30 + d2)>>' or a rank-reduced version. (mismatch of result layout) 
    %4 = memref.subview %3[%c0, 0, 0] [1, 30, 30] [1, 1, 1] : memref<1x30x30xi32> to memref<30x30xi32>
         ^
foo.mlir:6:10: note: see current operation: %2 = "memref.subview"(%1, %0) {operand_segment_sizes = dense<[1, 1, 0, 0]> : vector<4xi32>, static_offsets = [-9223372036854775808, 0, 0], static_sizes = [1, 30, 30], static_strides = [1, 1, 1]} : (memref<1x30x30xi32>, index) -> memref<30x30xi32>

Also static_offsets being minus seems very weird

2

I get the same err, did you solve this problem？

3

Ugh sorry for delay, this passed under my radar until now.

The verifier does not try to do the job of the folder, so it does not go inside value to detect that %c0 is actually 0.
As a consequence it expects the offset to be dynamic.

The following IR works for me:

module  {
  func.func private @foo(memref<30x30xi32, strided<[30, 1], offset: ?>>) -> i32
  func.func @main() {
    %c0 = arith.constant 0 : index
    %3 = memref.alloca() : memref<1x30x30xi32>
    %4 = memref.subview %3[%c0, 0, 0] [1, 30, 30] [1, 1, 1] 
		  : memref<1x30x30xi32> to memref<30x30xi32, strided<[30, 1], offset: ?>>
    %7 = call @foo(%4) : (memref<30x30xi32, strided<[30, 1], offset: ?>>) -> i32
    return
  }
}
4

That is unfortunate but I wasn’t aware of anything we could do better at the time of introduction, so usual tradeoffs apply.

@Mogball @mehdi_amini do you know if we have support in the parser/printer infra these days to plug in such a sentinel value?

We use min<int64_t> to encode ?.

From the beginning of time, memref used -1 to encode a ? for the shape and it was very hard to remove that hardcoding across multiple internal projects we were not following (@pifon2a has all the gory details).

min<int64_t> is not a footgun from that perspective as it is very hard to misuse into something correct. Still, would be nice to have say DenseI64ArrayAttr have a special encoding for unknown/dynamic.
I’d expect us to want an enum just in case we want to support other things in the future …

Thoughts?

5

Are we talking about printing/parsing exclusively?
We could do something about it, instead of using DenseI64ArrayAttr we could need a thin wrapper using another attribute that would print differently (because DenseI64ArrayAttr can be used in context where the content isn’t meant to represent a “dimension”).

Side note: dimension list is what motivated DenseI64ArrayAttr initially, the first version of the patch even has this attribute named DimensionListAttr originally.