[mlir][nfc] Clarify the limitation on scalable vectors

[awarzynski]

Summary

When converting/lowering to LLVM, MLIR does not support n-D scalable
vectors at the moment. This patch simply clarifies an assert that's
meant to catch such unsupported cases.

Note that we may want to relax this condition in the future.

[dcaballe]

Thanks!

[c-rhodes]

mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp

473–476

this type conversion runs before the pattern I'm adding in D152508 and this assert this will break that.

[awarzynski]

I've done a bit of digging and summarized my findings here: https://github.com/openxla/iree/issues/14294.

Tl;Dr It's a bit tricky and convoluted :) Basically, we need to find a way to prevent LLVM type converter to treat scalable 2-d vectors _when_ targeting ArmSME. For SME such types are legal, though obfuscated. Importantly, LLVM does not support such types and we need to make sure that the "SME lowering" takes care of everything.

I am about to send an update so that you can see what solution I came up with.

[awarzynski]

Rebase on top of D154867, add a workaround for SME

[c-rhodes]

mlir/include/mlir/Conversion/LLVMCommon/LoweringOptions.h

37 ↗

(On Diff #539140)

How about flipping to disable since that's what the intention is here? And also add "type"

mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp

460

I think it's worth highlighting here why this is, i.e. LLVM doesn't support arrays for scalable vectors.

474

nit: use getRank

mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVMPass.cpp

100 ↗

(On Diff #539140)

nit: introduced

103–104 ↗

(On Diff #539140)

nit: I would rephrase to mentioned the problem "The LLVM type converter converts vectors of rank > 1 to arrays of vectors, but LLVM doesn't support arrays of scalable vectors."

105–106 ↗

(On Diff #539140)

how about this, and moving it above where converter is created?

[awarzynski]

@c-rhodes , thanks for the review! I am realising that rather than extending LLVMConverter, we could just create a custom type converter for SME that inherits from LLVMConverter and effectively disables conversions for VectorType: https://reviews.llvm.org/D155365. This way LLVMConverter remains intact, which IMHO is a cleaner way to approach this. I will updating this shortly.

[awarzynski]

Move the TypeConversion "fix" to https://reviews.llvm.org/D155365, address Cullen's comments

[tschuett]

Why is there no SVESME.md on mlir-www with the current status and supported features?

[awarzynski]

In D154302#4503249, @tschuett wrote:

Why is there no SVESME.md on mlir-www with the current status and supported features?

What do you mean by mlir-www? https://mlir.llvm.org/? I don't see any dedicated space there to track WIP projects. A lot of MLIR folks use Discourse to communicate updates and that's what we try to do on semi-regular basis, see:

• https://discourse.llvm.org/t/vector-to-armsme/
• https://discourse.llvm.org/t/an-update-on-armsme-dialect-and-proposed-next-steps/

IMHO, this patch is too small to deserve a dedicated post.

mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp

473–476

Fixed by introducing a custom TypeConverter in: https://reviews.llvm.org/D155365

[tschuett]

The git repository behind mlir.llvm.org:
https://github.com/llvm/mlir-www

[c-rhodes]

LGTM cheers

mlir/lib/Conversion/LLVMCommon/TypeConverter.cpp

Type LLVMTypeConverter::convertMemRefToBarePtr(BaseMemRefType type) {
   return getPointerType(elementType, *addressSpace);
 }
 
 /// Convert an n-D vector type to an LLVM vector type:
 ///  * 0-D `vector<T>` are converted to vector<1xT>
 ///  * 1-D `vector<axT>` remains as is while,
 ///  * n>1 `vector<ax...xkxT>` convert via an (n-1)-D array type to
 ///    `!llvm.array<ax...array<jxvector<kxT>>>`.
+/// As LLVM does not support arrays of scalable vectors, it is assumed that
+/// scalable vectors are always 1-D. This condition could be relaxed once the
+/// missing functionality is added in LLVM
 Type LLVMTypeConverter::convertVectorType(VectorType type) {
   auto elementType = convertType(type.getElementType());
   if (!elementType)
     return {};
   if (type.getShape().empty())
     return VectorType::get({1}, elementType);
   Type vectorType = VectorType::get(type.getShape().back(), elementType,
                                     type.getScalableDims().back());
   assert(LLVM::isCompatibleVectorType(vectorType) &&
          "expected vector type compatible with the LLVM dialect");
-  assert((type.isScalable() == type.allDimsScalable()) &&
-         "expected scalable vector with all dims scalable");
+  assert(
+      (!type.isScalable() || (type.getRank() == 1)) &&
+      "expected 1-D scalable vector (n-D scalable vectors are not supported)");
   auto shape = type.getShape();
   for (int i = shape.size() - 2; i >= 0; --i)
     vectorType = LLVM::LLVMArrayType::get(vectorType, shape[i]);
   return vectorType;
 }
 
 /// Convert a type in the context of the default or bare pointer calling
 /// convention. Calling convention sensitive types, such as MemRefType and
 /// UnrankedMemRefType, are converted following the specific rules for the