This is an archive of the discontinued LLVM Phabricator instance.

[mlir][LLVM] Introduce an intrinsic for llvm.matrix.multiply
AbandonedPublic

Authored by nicolasvasilache on Mar 4 2020, 3:16 PM.

Details

Summary

This revision adds the first intrinsic for llvm.matrix.multiply.
This uses the more general LLVM_OneResultOp for now since the goal is to use the
specific Matrix builders that @fhahn has created recently.

When piped through:

opt -O3 -enable-matrix | llc -O3 -march=x86-64 -mcpu=skylake-avx512

this has been verified to generate ymm instructions.

Additional function attribute support will be needed to generate proper zmm instructions but at least things run end to end.

Benchmarking will be provided separately with the experimental metaprogramming ModelBuilder tool when ready.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2020, 3:16 PM
nicolasvasilache added a subscriber: simoll.

Note the TODO cleanup, please hold off on reviewing for now, this is more of a heads up that this is coming.
We'll also need to pipe this through the JIT properly for exec.

Also + @fhahn and @simoll in case this is interesting.

mehdi_amini added inline comments.Mar 5 2020, 12:06 AM
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
777

Seems like some cleanup needed right now? You likely wrote this internally

ftynse requested changes to this revision.Mar 5 2020, 1:20 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
775

Can you elaborate?

801

Could have at least have a textual description to understand what ABMNK mean? These will be the names of accessor functions as well...

801

Also, any reason to chose I32 attribute?

819

I suppose the Op arguments are also in that order, so it would make sense to reorder them in Arguments<>.

823

This looks sufficiently long to go to .cpp instead.

828

This builder seems to ignore operands. I would also expect such a straightforward builder to be autogenerated.

836

Could we use a more conventional function-type notation: '(' type($A) ',' type($B) ') -> ' type($res) (or maybe there's support for that directly in the formatter)

This revision now requires changes to proceed.Mar 5 2020, 1:20 AM
nicolasvasilache retitled this revision from [mlir] Introduce an intrinsic for llvm.matrix.multiply to [mlir]WIP - Introduce an intrinsic for llvm.matrix.multiply.Mar 5 2020, 11:10 AM
nicolasvasilache marked 8 inline comments as done.

Cleanup and simplify now that I can use MatrixBuilder.

format

mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
801

to match the LLVM impl: https://reviews.llvm.org/D70456

nicolasvasilache retitled this revision from [mlir]WIP - Introduce an intrinsic for llvm.matrix.multiply to [mlir][LLVM] Introduce an intrinsic for llvm.matrix.multiply.Mar 5 2020, 2:29 PM
nicolasvasilache edited the summary of this revision. (Show Details)
nicolasvasilache marked an inline comment as done.
aartbik accepted this revision.Mar 5 2020, 2:52 PM
aartbik added inline comments.
mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
790

In the longer run, do we want any sanity checks on the passed in rows/columns and types of the operands?

nicolasvasilache abandoned this revision.Mar 6 2020, 5:36 AM