This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Bufferize] Fix incorrect bufferization of rank-reducing tensor ops.
ClosedPublic

Authored by nicolasvasilache on Jan 9 2022, 1:18 PM.

Details

Summary

This revision fixes SubviewOp, InsertSliceOp, ExtractSliceOp construction during bufferization
where not all offset/size/stride operands were properly specified.

A test that exhibited problematic behaviors related to incorrect memref casts is introduced.
Init tensor optimization is disabled in teh testing func bufferize pass.

Diff Detail

Event Timeline

nicolasvasilache requested review of this revision.Jan 9 2022, 1:18 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 9 2022, 1:18 PM
springerm accepted this revision.Jan 10 2022, 12:18 AM
springerm added inline comments.
mlir/include/mlir/Interfaces/ViewLikeInterface.td
488–489

Maybe worth noting that the missing dims are assumed to be "top" (least significant?) dims, i.e., [offsets.size(); rank). Or is this obvious from the context?

mlir/lib/Dialect/Linalg/ComprehensiveBufferize/BufferizationInterfaceImpl.cpp
81–83 ↗(On Diff #398456)

Could we fix this by always casting all memrefs to their "most dynamic" variants? I.e., dynamic dims and dynamic offsets + strides in the affine map. Then let canonicalization clean it up.

mlir/test/Dialect/Linalg/comprehensive-module-bufferize-memref-cast-bug.mlir
1 ↗(On Diff #398456)

Should we merge these into a single file for "regression tests"?

This revision is now accepted and ready to land.Jan 10 2022, 12:18 AM
nicolasvasilache marked an inline comment as done.

Rebase address and cleanup

nicolasvasilache marked 2 inline comments as done.Jan 10 2022, 7:06 AM
nicolasvasilache added inline comments.
mlir/lib/Dialect/Linalg/ComprehensiveBufferize/BufferizationInterfaceImpl.cpp
81–83 ↗(On Diff #398456)

The issue was related to rank-reduction due to the init tensor simplification and the missing rank-reducing behavior in the linalgimpl.
We may still want to do what you suggest in the future but this was a legitimate bug.

mlir/test/Dialect/Linalg/comprehensive-module-bufferize-memref-cast-bug.mlir
1 ↗(On Diff #398456)

yes it was just for the purpose of testing for now, the PR was marked WIP.

nicolasvasilache retitled this revision from [mlir][Bufferize][WIP] Fix incorrect bufferization of rank-reducing tensor ops. to [mlir][Bufferize] Fix incorrect bufferization of rank-reducing tensor ops..Jan 10 2022, 7:06 AM
nicolasvasilache edited the summary of this revision. (Show Details)
nicolasvasilache marked 2 inline comments as done.

Reasbe , clean

This revision was landed with ongoing or failed builds.Jan 10 2022, 7:15 AM
This revision was automatically updated to reflect the committed changes.