Page MenuHomePhabricator

[mlir] Introduce `linalg.tiled_yield` terminator for `linalg.tiled_loop`.
Needs RevisionPublic

Authored by pifon2a on Jul 15 2021, 7:44 AM.

Diff Detail

Event Timeline

pifon2a created this revision.Jul 15 2021, 7:44 AM
pifon2a requested review of this revision.Jul 15 2021, 7:44 AM
pifon2a edited the summary of this revision. (Show Details)Jul 15 2021, 7:51 AM

Just some comments as I looked at it anyway.

mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
754

This comment is off. There is no enclosing linalg generic op. Maybe explain that it updates the part of the enclosing tiled_loop ops result specified by the second operand with the values from the first operand.

mlir/lib/Dialect/Linalg/Transforms/ComprehensiveBufferize.cpp
574

If the assumption is that it always is canonicalized away to an in-place update, then the terminator does not have any of these effects. However, I don't know this part well enough to judge whether it models the naive state or the expected bufferized behavior.

mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
956 ↗(On Diff #358963)

Debuggin left-over.

mlir/lib/Dialect/Linalg/Transforms/Tiling.cpp
307–308

Comment is now off

pifon2a updated this revision to Diff 359737.Jul 19 2021, 3:57 AM
pifon2a marked 3 inline comments as done.

Address the comments.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 19 2021, 5:17 AM
This revision was automatically updated to reflect the committed changes.
pifon2a reopened this revision.Jul 19 2021, 7:16 AM
pifon2a updated this revision to Diff 359782.Jul 19 2021, 7:17 AM

Add a pattern to fold tensor.cast into tiled_yield.

mehdi_amini requested changes to this revision.Jul 19 2021, 5:22 PM

(Marking as blocking while we're discussing on the RFC)

This revision now requires changes to proceed.Jul 19 2021, 5:22 PM