Page MenuHomePhabricator

[mlir][sparse] use shared util for DimOp generation
ClosedPublic

Authored by aartbik on Aug 18 2021, 10:55 AM.

Details

Summary

This shares more code with existing utilities. Also, to be consistent,
we moved dimension permutation on the DimOp to the tensor lowering phase.
This way, both pre-existing DimOps on sparse tensors (not likely but
possible) as well as compiler generated DimOps are handled consistently.

Diff Detail

Event Timeline

aartbik created this revision.Aug 18 2021, 10:55 AM
aartbik requested review of this revision.Aug 18 2021, 10:55 AM
aartbik updated this revision to Diff 367292.Aug 18 2021, 12:02 PM

rebase with main

bixia added inline comments.Aug 18 2021, 4:03 PM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorConversion.cpp
243

Looks good to me. I only have one question. If the dimension 'idx' in the tensor type has a constant value, can we generate a constant instead of a call to sparseDimSize?

aartbik marked an inline comment as done.Aug 18 2021, 4:17 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorConversion.cpp
243

Yes, but that is already done by earlier folding (either in pure MLIR for non-annotated tensors, or in sparse compiler with the call to createOrFoldDimOp).

aartbik marked an inline comment as done.Aug 18 2021, 4:35 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorConversion.cpp
243

Ah, but your question made me realize that MLIR's pure folding needs to be made aware of the permutation too, of course. I will fix that there.

bixia accepted this revision.Aug 18 2021, 5:05 PM
This revision is now accepted and ready to land.Aug 18 2021, 5:05 PM
This revision was automatically updated to reflect the committed changes.