Page MenuHomePhabricator

[mlir][splitting std] move 2 more ops to `tensor`
ClosedPublic

Authored by silvas on Jan 19 2021, 12:41 PM.

Details

Summary
  • DynamicTensorFromElementsOp
  • TensorFromElements

Diff Detail

Event Timeline

silvas created this revision.Jan 19 2021, 12:41 PM
silvas requested review of this revision.Jan 19 2021, 12:41 PM
rriddle accepted this revision.Jan 19 2021, 12:46 PM
rriddle added inline comments.
mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td
13

What are you using from here?

mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
246

getDefiningOp<FromElementsOp>?

294

We should move this to use the assemblyFormat.

mlir/lib/Dialect/Tensor/Transforms/PassDetail.h
12

Is this include actually necessary?

This revision is now accepted and ready to land.Jan 19 2021, 12:46 PM
silvas updated this revision to Diff 317678.Jan 19 2021, 1:42 PM
silvas marked an inline comment as done.

Address River's comments.

silvas updated this revision to Diff 317683.Jan 19 2021, 1:46 PM

Use forward decl.

silvas marked an inline comment as done.Jan 19 2021, 1:47 PM
silvas added inline comments.
mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td
13

ReturnLike for YieldOp

mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
294

Looks like it doesn't work

let arguments = (ins Variadic<Index>:$dynamicExtents);
let results = (outs AnyRankedTensor:$result);
let regions = (region SizedRegion<1>:$body);
let assemblyFormat = "$dynamicExtents $body attr-dict";

$dynamicExtents is not buildable. (I guess Variadic<Index> doesn't get inferred properly?)

mlir/lib/Dialect/Tensor/Transforms/PassDetail.h
12

switched to forward decl.

silvas marked an inline comment as done.Jan 19 2021, 1:47 PM
silvas added inline comments.
mlir/lib/Dialect/Tensor/IR/TensorOps.cpp
294

Sorry, ignore this.

rriddle added inline comments.Jan 19 2021, 1:51 PM
mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td
13

Ah right, I was looking for the interfaces and completely forgot ReturnLike was in there too.

This revision was landed with ongoing or failed builds.Jan 19 2021, 1:54 PM
This revision was automatically updated to reflect the committed changes.
silvas added inline comments.Jan 19 2021, 1:59 PM
mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td
13

Me too. When I factored it out, I got errors about ReturnLike being missing and it took me some time to remember that it was in there as well (in the end I just grepped for ReturnLike and found it "oh... yeah, I guess it makes sense for it to be there").

rriddle added inline comments.Jan 19 2021, 2:17 PM
mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td
210

I don't think we should require this. Can you file a starter bug to look into this(if there isn't one already)? It'd be nice to not require these kind of things, especially because it looks broken in this case (YieldOp expects an argument).

silvas added inline comments.Jan 19 2021, 3:30 PM
mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td
210