Page MenuHomePhabricator

[mlir] Extend `tensor.from_elements` to support N-D case.
ClosedPublic

Authored by pifon2a on Dec 15 2021, 12:04 PM.

Diff Detail

Event Timeline

pifon2a created this revision.Dec 15 2021, 12:04 PM
pifon2a requested review of this revision.Dec 15 2021, 12:04 PM
herhut added inline comments.Dec 16 2021, 4:54 AM
mlir/include/mlir/Dialect/Tensor/IR/TensorOps.td
322

Can you also mention how the order of elements relates to elements in the tensor?

mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp
198

Debugging leftover?

198–199

Could this use the overload that just takes the values?

571

Here, too.

571

Could this use the overload that just takes extenTensor?

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

This needs to be the stride. So you can compute flatIndex *= tensorType.getDimSize(i) and then do flatIndex += index.getSExtValue.

mlir/lib/Dialect/Tensor/Transforms/Bufferize.cpp
85

Would it be easier to always allocate a 1d memref, write the values and then reshape it into the final shape?

pifon2a updated this revision to Diff 394837.Dec 16 2021, 5:42 AM
pifon2a marked 7 inline comments as done.

Address the comments.

mlir/lib/Conversion/ShapeToStandard/ShapeToStandard.cpp
198–199

only if we are absolutely sure that the elements are not empty. Unfortunately, there is this ugly use case:

%0 = tensor.from_elements : tensor<0xindex>
571

There is a builder that does it. But we have to be sure that extentValues are not empty.

mlir/lib/Dialect/Tensor/Transforms/Bufferize.cpp
85

It would be the same number of stores + additional reshape that might break some canonicalizations. I think the best variant would be to have a memref.from_elements and then we could lower it to llvm creating a necessary descriptor and linearly populating the underlying buffer.

herhut accepted this revision.Dec 16 2021, 5:51 AM
This revision is now accepted and ready to land.Dec 16 2021, 5:51 AM
This revision was landed with ongoing or failed builds.Dec 16 2021, 5:59 AM
This revision was automatically updated to reflect the committed changes.