Page MenuHomePhabricator

[mlir] Fix incorrect indexing of subview in DimOp folding.
ClosedPublic

Authored by nicolasvasilache on May 15 2020, 9:19 AM.

Details

Summary

DimOp folding is using bare accesses to underlying SubViewOp operands.
This is generally incorrect and is fixed in this revision.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2020, 9:19 AM
andydavis1 accepted this revision.May 15 2020, 9:54 AM
This revision is now accepted and ready to land.May 15 2020, 9:54 AM
mravishankar accepted this revision.May 15 2020, 9:58 AM
mravishankar added inline comments.
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
1339–1341

Does this has to be an assert? Is it possible that the subview op has been canonicalized, but the dim op hasnt in which case the subview op will have a static size. You can just return {} if it is not dynamic for now.

nicolasvasilache marked 2 inline comments as done.May 15 2020, 10:39 AM
nicolasvasilache added inline comments.
mlir/lib/Dialect/StandardOps/IR/Ops.cpp
1339–1341

I think so: if the size was static we would have been done at the beginning gettin the information from the memref type.
If not, then we expect to have a dynamic index.
Otherwise the subview itself would be inconsistent and we'd definitely want to fail hard there.

I note however I also need to update the comment.

nicolasvasilache marked an inline comment as done.

Cleanups.

This revision was automatically updated to reflect the committed changes.