[RFC] MemRefType affine maps - list vs single item

Why? It’s clearly indicated in the type: memref<5xf32, affine_map<(d0) -> (d0 + 5)>> has a static offset of 5, memref<5xf32, affine_map<(d0)[s0] -> (d0 + s0)>> has a dynamic offset expressed as symbol s0.

It can’t be. You can have a memref with dynamic strides and a dynamic offset, and the linear offset is computed as “aligned_ptr + offset + i * stride”, it is only equivalent to ArrayRef if we statically know that offset=0 and stride=1. We do for memref<10xf32> and memref<5xf32, affine_map<(d0) -> (d0 + 5)>>, but we don’t for memref<5xf32, offset: ?, strides: [?]> (alternatively memref<5xf32, (d0)[s0, s1] -> (d0 * s1 + s0)>), which is perfectly valid type.

The annoying part is that one can’t pass a memref with offset where a memref with no offset (type defaults to static 0 offset) is expected, but they are not the same type. The workaround is to take the most dynamic type and then to propagate constants in the type or specialize when necessary.

The producing operation is only visible in the translation unit it is defined in in the best case, or in a function / other isolated-from-above operation in the worst case. The information about it will be lost at function call boundary in most cases. We consider it generally useful for aliasing analysis: it’s better to know that a value is base+offset and have some information about the offset than to deal with arbitrary pointers. I don’t think it’s actively exercised upstream though.

There is always a trade off between keeping such things in operation chains and putting them in the type. I think we ultimately need both.

Currently on vacation so replying very briefly.
It is true that the static offset so far has not really paid for itself and could technically be elided.

Initially the idea was to encode the shift of multiple independent views inside a larger backing allocation. There is currently no use of that and it is unclear it will materialize.

So if there is a case for it I wouldn’t be opposed from eliding the offset from the type. The interpretation would be that the offset is always ‘?’.

This could potentially lose alignment information though and should be evaluated if there is a compelling reason to drop it from the type.

I guess this is caused by current StridedMemRefType implementation. But what the reason to store offset as separate field? Why not just use pointer arithmetic?

// Current implementation

template <typename T, int N>
struct StridedMemRefType {
  T *data;
  int64_t offset;
  int64_t sizes[N];
  int64_t strides[N];
};

// "%1 = memref.subview %0" can be represented as
StridedMemRefType<...> var1;
var1.data = var0.data;
var1.offset = <1D offset from memref.subview>
var1.sizes = <sizes from memref.subview>
var1.strides = var0.strides;

// It could also be implemented as

template <typename T, int N>
struct StridedMemRefType {
  T *data;
  int64_t sizes[N];
  int64_t strides[N];
};

// "%1 = memref.subview %0" can be represented as
StridedMemRefType<...> var1;
var1.data = var0.data + <1D offset from memref.subview>; // add offset directly to the new pointer
var1.sizes = <sizes from memref.subview>
var1.strides = var0.strides;

In the second case offset is not a part of type definition. IMHO, this simplifies IR, since you don’t need to care about type difference due to offset.

Not really. Although we did think about connecting the strided memref to C, the type design isn’t (and shouldn’t be) driven by the implementation of a helper class in a test tool.

Nicolas answered this above. Why not just use pointer arithmetic everywhere and ditch memref entirely? :wink:

And then you need to convert this to the LLVM struct that does have a field for offset… Certainly, you can set it to zero, but converting from MLIR to the same LLVM struct wouldn’t necessarily. So we will have different representations for the same object based on its provenance. I feel strongly negatively about such small discrepancies because they turn writing interface into a walk through a minefield, as if it wasn’t error-prone enough already.

The main design decision for this specific case was to have portable malloc/free for which we keep the base pointer. This can still be done while eliding the offset from the memref type in MLIR. Whether the changes are worth it is another discussion. As you mentioned we can always erase to ‘?’ at the function boundary which is the only place where this should matter.

Many other slight changes and design tradeoffs are possible and have non trivial implications on codegen and the rest of the world. As of now the current design is stable and enough independent things that intersect work. Potential changes need stronger motivation and discussion of implications.

In the end, as Alex mentions we can always unpack every index and go back to linearized pointer arithmetic. Atm this is kept opaque until in conversion to llvm. There are good reasons to unpack some if this like unification with spirv but we also want to keep codegen structured.

@ftynse I’ve implemented your proposal (storing layout as generic attribute, which implements special attribute interface) - ⚙ D111553 [mlir][RFC] Refactor layout representation in MemRefType. Could you please take a look at this patch?