This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Linalg] Refactor padding hoisting - NFC
ClosedPublic

Authored by nicolasvasilache on Sep 24 2021, 6:43 AM.

Details

Summary

This revision extracts padding hoisting in a new file and cleans it up in prevision of future improvements and extensions.

Diff Detail

Event Timeline

nicolasvasilache requested review of this revision.Sep 24 2021, 6:43 AM
ftynse accepted this revision.Sep 24 2021, 9:37 AM
ftynse added inline comments.
mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp
44

Nit: documentation for top-levle entity?

49

Nit: /// here and below.

104

I suspect you need to wrap this in LLVM_DEBUG() as well or get an unused variable warning/error.

120

Ditto.

This revision is now accepted and ready to land.Sep 24 2021, 9:37 AM
springerm added inline comments.Sep 24 2021, 7:11 PM
mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp
61

Tiling can generate 4 kinds of loops: scf.for, scf.parallel, affine.for, tiled_loop.

Will this always stay scf.for or do we want to extend this in the future?

122

Is this to prevent the pattern from being applied to other pad_tensor ops (that may be created by the user)?

I'm wondering if we could alternatively start with a pointer to a tiled Linalg op (which we should have after tiling) and get to the pad_tensor ops by checking its inputs. (Instead of looking for pad_tensor ops in the FuncOp.)

nicolasvasilache marked 6 inline comments as done.Sep 27 2021, 2:39 AM
nicolasvasilache added inline comments.
mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp
61

The code used to be written for more loop types but in practice it is unsupported and premature generalization.
Multi-loops are quite annoying because they may also require splitting in smaller bands of loops, so this is not yet supported.
Loops with parallel semantics have extra considerations re what can be reused or not

104

thanks for catching!

122

This is because hoisting the output padding is not yet supported (i.e. there is something to be done for the writeback that isn't implemented).
Additionally, hoisting of output is generally handled at the vector level when the read/write turn into loop carried dependences.

nicolasvasilache marked 3 inline comments as done.

Address comments.

This revision was landed with ongoing or failed builds.Sep 27 2021, 2:50 AM
This revision was automatically updated to reflect the committed changes.