This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Vector] Add patterns to reorder elementwise ops and broadcast/transpose ops.
ClosedPublic

Authored by hanchung on Mar 1 2022, 12:04 PM.

Details

Summary

In quantized comutation, there are casting ops around computation ops.
Reorder the ops to make reduce-to-contract actually work.

Diff Detail

Event Timeline

hanchung created this revision.Mar 1 2022, 12:04 PM
hanchung requested review of this revision.Mar 1 2022, 12:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 12:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 12:49 PM
ThomasRaoux added inline comments.Mar 2 2022, 11:52 AM
mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
1033–1034

you can just use op.getType()?

1061–1063

same comment, you can use op.getType()?

2669–2684

can we use CastOpInterface instead of listing all the ops explicitly. It is probably fine to apply this to all unary ops as well otherwise.

mlir/test/Dialect/Vector/vector-reduce-to-contract.mlir
97

nit: can you add the return type

hanchung updated this revision to Diff 412859.Mar 3 2022, 4:16 PM
hanchung marked 4 inline comments as done.

Address comments

mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
1033–1034

ah, good point!

bondhugula added inline comments.
mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
1012

Please add a line for why this would be the canonical form (along the lines of your commit summary) -- this isn't obvious at all.

1038

Likewise.

1049

Use named accessor instead of getOperand(0)?

1049

I don't see where the template type here is.

hanchung updated this revision to Diff 413557.Mar 7 2022, 10:44 AM
hanchung marked 3 inline comments as done.

Add more comments and remove template types

ThomasRaoux accepted this revision.Mar 7 2022, 11:14 AM
ThomasRaoux added inline comments.
mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
1019–1023

nit: having the contract here may be a bit confusing as the pattern doesn't need the contract to kick in. Overall re-ordering the broadcast allows to do less work so it is usually better.

This revision is now accepted and ready to land.Mar 7 2022, 11:14 AM
hanchung updated this revision to Diff 413580.Mar 7 2022, 11:57 AM
hanchung marked an inline comment as done.

Update comments

mlir/lib/Dialect/Vector/Transforms/VectorTransforms.cpp
1049

I don't find a named accessor. Seems that the CastOpInterface does not have named accessor?