This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Vector] Add a vector.matrix_multiply op on 1-D vectors
ClosedPublic

Authored by nicolasvasilache on Mar 6 2020, 2:49 PM.

Details

Summary

This op mirrorrs the llvm.intr counterpart and allows lowering + type conversions in a progressive fashion.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2020, 2:49 PM
aartbik added inline comments.Mar 6 2020, 5:35 PM
mlir/include/mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h
20

why is this in its own population (instead of adding it to the general VectorToLLVM)?
I don't object, but just curious. I can see isolated testing, but you don't seem to exploit that

mlir/include/mlir/Dialect/VectorOps/VectorOps.td
1343

Some parts of this comments could be moved into description (e.g. for some others, we also say why we picked it, like being close to the llvm implementation)

1362

can we get a summary and description please? we need to keep our docs a bit up to date ;-)

nicolasvasilache marked 3 inline comments as done.

Address review comments, fix incorrect rhs_columns attribute name.

nicolasvasilache marked an inline comment as done.Mar 6 2020, 8:53 PM
nicolasvasilache added inline comments.
mlir/include/mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h
20

Since lowering matrix intrinsics in LLVM requires a special flag, it makes sense to group those patterns together.
Updated the doc to reflect this.

nicolasvasilache marked an inline comment as done.Mar 6 2020, 8:53 PM

Update comments.

mehdi_amini added inline comments.Mar 6 2020, 9:35 PM
mlir/include/mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h
20

(Typo: missing space in wheninvoking)

Why do we need a special flag for matrix lowering?

nicolasvasilache marked 2 inline comments as done.Mar 7 2020, 8:29 AM
nicolasvasilache added inline comments.
mlir/include/mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h
20

Not sure, @fhahn ?

nicolasvasilache marked an inline comment as done.

Typo.

aartbik accepted this revision.Mar 9 2020, 9:45 AM
aartbik added inline comments.
mlir/include/mlir/Conversion/VectorToLLVM/ConvertVectorToLLVM.h
20

typo: wheninvoking -> when invoking

mlir/include/mlir/IR/OpBase.td
490 ↗(On Diff #248934)

rebase this

nicolasvasilache marked 2 inline comments as done.Mar 9 2020, 10:33 AM
This revision was not accepted when it landed; it landed in state Needs Review.Mar 9 2020, 10:47 AM
This revision was automatically updated to reflect the committed changes.