I’d like to build on https://discourse.llvm.org/t/rfc-ir-permit-load-store-alloca-for-struct-of-the-same-scalable-vector-type by proposing the scalable vector restrictions be further relaxed to allow arrays of scalable vectors. Although my motivation comes from MLIR where high level data structures are often decomposed into arrays of vectors I feel the current restriction is artificial because arrays of scalable vectors are another example of “aggregate types with homogenous scalable vector types” the previous proposal sort to enable.
From an implementation point of view the existing struct support makes this easy (https://reviews.llvm.org/D158517), only requiring fixes to the ArrayType side of some issues the previous work hit with StructType, namely InstCombine unpacking of aggregate types and strengthening of the Verifier.
Are there good reasons to maintain the current restriction?
Thank you so much for working on this!
Big +1 from me.
As you point out, arrays of vectors are very common in MLIR. That’s primarily because many transformations are done using omnipresent highly dimensional Vectors/Tensors, e.g.
These vectors are then lowered to (nested) arrays of vectors - see MLIR Vector dialect - llvm level.
At the moment, missing support for arrays of scalable vectors in LLVM is a blocker for us to support scalable auto-vectorisation in Linalg:
While we could think of a workaround, having support for arrays of scalable vectors would naturally complement the design of the Vector dialect in MLIR.
Given how non-intrusive and beneficial this change is going to be, I strongly support it.
CC @dcaballe @nicolasvasilache
thanks for looking into this.
I don’t think there is a compelling reason to not allow arrays of scalable vectors because as you mention they’re aggregates of homogeneous scalables as well. I support your proposal.
Just to confirm, I understand nested arrays like
[2 x [2 x <vscale x 4 x float>]] would be OK as well, wouldn’t them?
I see no reason to disallow nested arrays and trying them with my current patch suggests they just work. That said, as was the case with scalable vectors, I anticipate they’ll be code paths that we only discover need to be updated once the first stage enablement is available.
I can’t think of a reason why arrays of length N should have different semantics to a struct containing exactly N elements of the same type. Therefore whatever works for those structs, should work for arrays. There’s probably a long tail of implementation divergence between the two.
I wonder whether changing ArrayType into a subtype of StructType would be the right thing to do in general. Take the perspective that array is a convenient representation of a struct containing N of the same element. More invasive that the proposal here but potentially saves work in the long run.
I’ve taken a look and I’m not sure what we’d really be saving by making
ArrayType a subtype of
StructType. From what I can see
ArrayType is a very simple extension of
StructType is more complex and includes methods that do not seem applicable to arrays. From a scalable vector support point of view the commonality between
StructType is restricted to just knowing when a type is scalable, which is also shared by the new target types and sits within
+1 to supporting arrays of scalable types. I don’t see any reason not to do this.
(Sorry, forgot I hadn’t already responded to this)