This is an archive of the discontinued LLVM Phabricator instance.

[mlir][vector] Add folder for ExtractStridedSliceOp
ClosedPublic

Authored by ThomasRaoux on Oct 20 2020, 11:35 PM.

Details

Summary

Add folder for the case where ExtractStridedSliceOp source comes from a chain of InsertStridedSliceOp. Also add a folder for the trivial case where the ExtractStridedSliceOp is a no-op.

Diff Detail

Event Timeline

ThomasRaoux created this revision.Oct 20 2020, 11:35 PM
ThomasRaoux requested review of this revision.Oct 20 2020, 11:35 PM
aartbik added inline comments.Oct 21 2020, 10:50 AM
mlir/lib/Dialect/Vector/VectorOps.cpp
1637

document function value

1640

give explicit integer decl rather than auto, here and below?

perhaps it is allowed by style, but you switch to explicit types below for int

1665

This is very subjective, so feel free to ignore but I always find

start <= offset && offset < end

easier to read :-)

1689

this really belongs to the else part right? this seems a bit strangely placed

Address review comments.

ThomasRaoux marked 4 inline comments as done.Oct 21 2020, 11:02 AM
aartbik accepted this revision.Oct 23 2020, 11:20 AM
aartbik added inline comments.
mlir/lib/Dialect/Vector/VectorOps.cpp
1651

not: bigger -> greater than (more mathy)

This revision is now accepted and ready to land.Oct 23 2020, 11:20 AM
ThomasRaoux marked an inline comment as done.

Thanks Aart!

This revision was automatically updated to reflect the committed changes.

LGTM, minor post-commit comments (just address and land if appropriate).
Thanks Thomas !

mlir/lib/Dialect/Vector/VectorOps.cpp
1633

s/o/of

1659

Put a TODO here for a more powerful analysis?

mlir/test/Dialect/Vector/canonicalize.mlir
170

Make this: %0 = ... offsets = [0, 1] to make the test a bit more interesting ?
The resulting offset should then be vector.extract_strided_slice ... offsets = [0, 0]