This is an archive of the discontinued LLVM Phabricator instance.

[mlir][ArmSME] Introduce custom TypeConverter for ArmSME
ClosedPublic

Authored by awarzynski on Jul 15 2023, 4:20 AM.

Details

Summary

At the moment, SME-to-LLVM lowerings relies entirely on
LLVMTypeConverter. This patch introduces a dedicated TypeConverter
that inherits from LLVMTypeConverter (it will also be used when
lowering ArmSME Ops to LLVM).

The new type converter merely disables lowerings for VectorType to
prevent 2-d scalable vectors (common in the context of ArmSME), e.g.

`vector<[16]x[16]xi8>`,

entering the LLVM Type converter. LLVM does not support arrays of
scalable vectors and hence the need for specialisation. In the case of
SME such types are effectively eliminated when emitting LLVM IR
intrinsics for SME.

Depends on D154867

Diff Detail

Event Timeline

awarzynski created this revision.Jul 15 2023, 4:20 AM
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
awarzynski requested review of this revision.Jul 15 2023, 4:20 AM
c-rhodes added inline comments.Jul 17 2023, 12:30 AM
mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVMPass.cpp
100

nit: this could be moved to if (armSME) {

mlir/lib/Dialect/ArmSME/Transforms/ArmSMETypeConverter.cpp
2

nit: update filename

16

does this disable all vector type conversion? Just wondering what happens for 1-d

Cheers for taking a look!

mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVMPass.cpp
100

That would be nice, but then it gets destroyed before it is used. I've not tried to investigate how to "move" ownership for these classes.

mlir/lib/Dialect/ArmSME/Transforms/ArmSMETypeConverter.cpp
16

does this disable all vector type conversion?

Yes, but only for SME Ops.

Just wondering what happens for 1-d

Once we hit a case like that, we could just copy LLVMTypeConverter::convertVectorType. I am not too worried about that just yet.

Update filename

c-rhodes accepted this revision.Jul 17 2023, 6:50 AM

LGTM cheers

mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVMPass.cpp
100

That would be nice, but then it gets destroyed before it is used. I've not tried to investigate how to "move" ownership for these classes.

No problem thanks for clarifying didnt consider that

mlir/lib/Dialect/ArmSME/Transforms/ArmSMETypeConverter.cpp
16

does this disable all vector type conversion?

Yes, but only for SME Ops.

Just wondering what happens for 1-d

Once we hit a case like that, we could just copy LLVMTypeConverter::convertVectorType. I am not too worried about that just yet.

We use 1-d scalable vectors in mlir/test/Integration/Dialect/Vector/CPU/ArmSME/vector-ops.mlir so I was wondering how this still worked, but of course it's because LLVMTypeConverter::convertVectorType doesn't actually do anything for 1-d 😄

This revision is now accepted and ready to land.Jul 17 2023, 6:50 AM
dcaballe accepted this revision.Jul 17 2023, 9:55 AM

Glad that this worked! Great!

mlir/lib/Conversion/VectorToLLVM/CMakeLists.txt
20

sort

This revision was landed with ongoing or failed builds.Jul 18 2023, 2:36 AM
This revision was automatically updated to reflect the committed changes.