This is an archive of the discontinued LLVM Phabricator instance.

[mlir][tensor] Add pattern to hoist extract/insert slice ops
Changes PlannedPublic

Authored by antiagainst on Sep 14 2022, 9:01 AM.

Details

Summary

This pattern is useful when we have extract->compute->insert op
patterns inside loops and the extract/insert offsets/sizes/strides
are all loop invariant. Such op patterns can be generated via,
for example, unrolling tiled loops.

Diff Detail

Event Timeline

antiagainst created this revision.Sep 14 2022, 9:01 AM
antiagainst requested review of this revision.Sep 14 2022, 9:01 AM

Add some comments

nicolasvasilache requested changes to this revision.Sep 19 2022, 3:16 AM

It would be good to unify the impl. and helper functions with lib/Dialect/Linalg/Transforms/Hoisting.cpp and/or bufferization and explicitly disallow having multiple different implementations (e.g. findMatchingExtractSlice or something very close must already exist somewhere, please refactor as you need to have 1 version that we can later evolve into an analysis or interface).

Adding @ThomasRaoux to weigh in since he wrote the hoisting.cpp part.

mlir/lib/Dialect/Tensor/Transforms/CMakeLists.txt
4

Please suffix this file name with Patterns -> HoistExtractInsertSlicePatterns.cpp

This revision now requires changes to proceed.Sep 19 2022, 3:16 AM

@nicolasvasilache: sure thing. I'll chat with Thomas to see how we can clean up these. I suspect we can disentangle the logic in Hoisting.cpp, which right now does both tensor and vector hoisting. We should be able to rely on this new pattern for tensor specifically and make vector side simpler. (Both side needs similar analysis, which we can also try to see how to unify.)

antiagainst planned changes to this revision.Oct 8 2022, 10:38 AM

Don't hoist if insert_slice has multiple uses

antiagainst planned changes to this revision.Oct 8 2022, 10:56 AM

@nicolasvasilache: sure thing. I'll chat with Thomas to see how we can clean up these. I suspect we can disentangle the logic in Hoisting.cpp, which right now does both tensor and vector hoisting. We should be able to rely on this new pattern for tensor specifically and make vector side simpler. (Both side needs similar analysis, which we can also try to see how to unify.)

@nicolasvasilache: I've uploaded D135546 to refactor the vector transfer hoisting logic to not doing tensor slice op hoisting; instead to use this as a prelimiary step as said in the above. I'll try to extract more common utilities to share next.

mravishankar resigned from this revision.Nov 16 2022, 9:22 PM
nicolasvasilache resigned from this revision.Feb 15 2023, 8:59 AM
antiagainst planned changes to this revision.Feb 24 2023, 2:12 PM