This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Vector] Add vector contraction to outerproduct lowering
ClosedPublic

Authored by nicolasvasilache on May 21 2020, 5:53 AM.

Details

Summary

This revision adds the additional lowering and exposes the patterns at a finer granularity for better programmatic reuse. The unit test makes use of the finer grained pattern for simpler checks.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2020, 5:53 AM
aartbik accepted this revision.May 21 2020, 11:42 AM
aartbik added inline comments.
mlir/include/mlir/Dialect/Vector/VectorTransforms.h
96

fields should follow methods (as you did below)

also , can this be a private field?

98

just curious, is the move away from matchAndRewrite in favor of match and rewrite intentional?

133

probably private?

160

fields after methods, probably private

mlir/lib/Dialect/Vector/VectorTransforms.cpp
1322

now these are static helpers, should we move them to the top of the file
(I have seen both styles, keep methods close to use, or move them to top)

I personally like at the top a bit better since it "encourages" other classes to start using them too, if they appear in the middle, they are less visible...

just my 2 cents

This revision is now accepted and ready to land.May 21 2020, 11:42 AM
nicolasvasilache marked 6 inline comments as done.May 26 2020, 6:08 AM
nicolasvasilache added inline comments.
mlir/include/mlir/Dialect/Vector/VectorTransforms.h
98

I've been using this for code reuse, see how matchAndRewrite below delegates to the other pattern's match method as a preprocessing filter.
Maybe in the future the better way is to use benefits but for now I kept it to the way I did it elsewhere.

nicolasvasilache marked an inline comment as done.

Address review.

This revision was automatically updated to reflect the committed changes.