Page MenuHomePhabricator

[mlir][linalg] Set explicit insertion point in pad_tensor patterns.
ClosedPublic

Authored by gysit on Jul 16 2021, 9:17 AM.

Details

Summary

Insert ops replacing pad_tensor in front of the associated tansfer_write / insert_slice op. Otherwise we may end up with invalid ir if one of the remaining tansfer_write / insert_slice operands is defined after the pad_tensor op.

Diff Detail

Event Timeline

gysit created this revision.Jul 16 2021, 9:17 AM
gysit requested review of this revision.Jul 16 2021, 9:17 AM
aartbik added inline comments.Jul 16 2021, 11:13 AM
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
877

it feels strange to have this right before a replace<> call
is this because of the insertions in the parameter, or the subsequence replace? (bit hard to judge without running this to see output)

can you please add a comment on the insertion point to explain why it is needed (since it is not a typical pattern in rewriting)

1027

same request, add a comment on why this is here

gysit added inline comments.Jul 16 2021, 1:07 PM
mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp
877

Good point.

The insertion point by default is after the matched operation AFAIK. Without setting the insertion point we would thus insert the new transfer operation right after the pad operation (the matched one). That means some of the transfer operands may be undefined / not dominating the use. For example, in the tests the newly created transfer op would be before the make_vector() and use its result.

Will add a comment in the source!

gysit updated this revision to Diff 359672.Jul 18 2021, 11:18 PM

Address comment.

This revision is now accepted and ready to land.Jul 19 2021, 12:50 AM