Page MenuHomePhabricator

[mlir][Linalg] Add a linalg.tensor_reshape to operate on tensors
ClosedPublic

Authored by nicolasvasilache on Apr 2 2020, 8:20 PM.

Details

Summary

This revision adds a tensor_reshape operation that operates on tensors.
In the tensor world the constraints are less stringent and we can allow more
arbitrary dynamic reshapes, as long as they are contractions.

The expansion of a dynamic dimension into multiple dynamic dimensions is under-specified and is punted on for now.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald Transcript
rriddle added inline comments.Apr 2 2020, 8:39 PM
mlir/lib/Dialect/Linalg/IR/LinalgOps.cpp
514

nit: llvm::is_contained(...)

rriddle resigned from this revision.Apr 2 2020, 8:39 PM
bondhugula requested changes to this revision.Apr 2 2020, 10:24 PM
bondhugula added a subscriber: bondhugula.
bondhugula added inline comments.
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
102–104

This looks problematic. Why add auto in-placing / other arg dependent in-place/out-of-place semantics? Instead, always make it out of place (other passes/utilities can in-place) or have separate ops for in-place and out-of-place?

mlir/test/Dialect/Linalg/invalid.mlir
512

The error message could be confusing - they don't have to be of the same shaped type - instead 'expected source and result to both be either tensor or memref'?

This revision now requires changes to proceed.Apr 2 2020, 10:24 PM
bondhugula added inline comments.Apr 2 2020, 10:36 PM
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
91

Update summary to include memref.

mravishankar added inline comments.Apr 2 2020, 11:08 PM
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
102–104

Could we have a utility function that codifies this semantics. This is a contract that backends have to enforce AFAICS., where such a utility function would be very useful.

nicolasvasilache marked 5 inline comments as done.

Address comments.

@bondhugula you're right a new op makes more sense.

nicolasvasilache retitled this revision from [mlir][Linalg] Extend linalg,reshape to operate on tensors to [mlir][Linalg] Add a linalg.tensor_reshape to operate on tensors.Apr 4 2020, 11:51 AM
nicolasvasilache edited the summary of this revision. (Show Details)
mravishankar accepted this revision.Apr 5 2020, 1:31 PM

Looked through this with the perspective of using this for fusing linalg operation on tensors. Seems fine to me, and if something needs changing when implementing that, will send that follow up CL.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 6 2020, 8:39 AM
This revision was automatically updated to reflect the committed changes.
antiagainst added inline comments.Apr 6 2020, 12:24 PM
mlir/include/mlir/Dialect/Linalg/IR/LinalgOps.td
140

s/sizes/dimensions/ ?

143

Nit: I think a reassociation is represented as an affine map attribute and all the reassociations as a whole are represented as the array attribute?