Page MenuHomePhabricator

[mlir] Purge `linalg.copy` and use `memref.copy` instead.
ClosedPublic

Authored by pifon2a on Jan 24 2022, 3:46 AM.

Diff Detail

Event Timeline

pifon2a created this revision.Jan 24 2022, 3:46 AM
pifon2a requested review of this revision.Jan 24 2022, 3:46 AM
pifon2a updated this revision to Diff 402463.Jan 24 2022, 3:57 AM

fix the flag.

pifon2a updated this revision to Diff 402467.Jan 24 2022, 4:01 AM

another flag.

Notes for reviewers:

  1. mlir/test/Dialect/Linalg/transform-patterns-matmul-to-vector.mlir was removed, since it has fusion on memrefs.
  2. Vectorization.cpp now contains a pattern to vectorize CopyOp in absence of LinalgOp interface.

Thanks @pifon2a, this was long overdue!

Please note the recent https://reviews.llvm.org/D117696 and that the situation is not 100% rosy as I have measure the memref.copy -> library call to be around 200x slower than a properly tiled and vectorized linalg.copy.
At this point memref.copy cannot tile and would significantly degrade perf in certain cases.
Let's iterate a bit on benchmarks before we land this if you don't mind.

Still this is a great step forward !

Thanks @pifon2a, this was long overdue!

Thanks yes,

I made a similar transition in the sparse compiler part a while back (because my understanding was that memref.copy would get all the attention over linalg.copy).
I hope we can migrate the performance into the new op soon!

nicolasvasilache accepted this revision.Jan 31 2022, 9:07 AM

Perf is good, thanks again @pifon2a !

This revision is now accepted and ready to land.Jan 31 2022, 9:07 AM
This revision was landed with ongoing or failed builds.Jan 31 2022, 9:26 AM
This revision was automatically updated to reflect the committed changes.