This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add shape.with_shape op
ClosedPublic

Authored by jpienaar on Jul 20 2020, 6:55 PM.

Details

Summary

This is an operation that can returns a new ValueShape with a different shape. Useful for composing shape function calls and reusing existing shape transfer functions.

Just adding the op in this change.

Diff Detail

Event Timeline

jpienaar created this revision.Jul 20 2020, 6:55 PM
Herald added a project: Restricted Project. · View Herald Transcript

Adding the op SGTM.

mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
435

Could we keep the op name and its textual form related? That makes it easier to match the two.

438

Can you be more precise what match means? Is the passed shape used verbatim (like a set_shape operation in a way)?

446

Nit: Drop one where.

462

I don't understand this comment. I think it answers my question from above but it is hard to parse.

frgossen added inline comments.Jul 21 2020, 2:00 AM
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
439

What does it men when shapes are non-conformant?

frgossen added inline comments.Jul 21 2020, 4:32 AM
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
438

It is only updated if the operand is a ValueShape. Otherwise, a new ValueShape is created?

455

Duplicate %%

mlir/test/Dialect/Shape/ops.mlir
185

Why shape_equal_shapes? This is essentially a join, is it?

191

Why shape_with_shape? The problem might just be that I don't fully understand the concept of ShapeValue.

jpienaar updated this revision to Diff 279539.Jul 21 2020, 8:45 AM
jpienaar marked 16 inline comments as done.

Fixing typos, adding sentence to ValueShape type to describe conformance, adding more comments

jpienaar added inline comments.Jul 21 2020, 8:59 AM
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td
435

Sorry, I had considered renaming it at the end and then decided it wasnt truly a refine (even though I found the naming better) but seems I forgot to rename back.

438

It is not really set: a new ValueShape is produced with the given shape, so effectively it is a copy and set.

438

A new ValueShape is always created, it is an SSA value.

439

shape_of(value) is equal to/conforms to shape at execution time. So the shape captured along with the value is consistent effectively.

462

Tried to clarify.

mlir/test/Dialect/Shape/ops.mlir
185

Correct, this is just function to invoke below, I just chose a simple one. I added a comment.

191

I was following the convention in this file op shape.X gets tested in func shape_X.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 31 2020, 2:47 PM
This revision was automatically updated to reflect the committed changes.