This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] Improve the implementation of sparse_tensor.new for the codegen path.
ClosedPublic

Authored by bixia on Feb 21 2023, 9:27 AM.

Details

Summary

Rewrite a NewOp into a NewOp of a sorted COO tensor and a ConvertOp for
converting the sorted COO tensor to the destination tensor type.

Codegen a NewOp of a sorted COO tensor to use the new bulk reader API and sort
the elements only when the input is not sorted.

Diff Detail

Event Timeline

bixia created this revision.Feb 21 2023, 9:27 AM
Herald added a project: Restricted Project. · View Herald Transcript
bixia requested review of this revision.Feb 21 2023, 9:27 AM
Peiming added inline comments.Feb 21 2023, 9:44 AM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
1323

is it a dimRank or lvlRank? If you are reading in a COO, it should be lvlRank here? @wrengr

aartbik added inline comments.Feb 21 2023, 10:52 AM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
1300

This needs a comment. All other new op situation are handled by the rewriting, and direct IR codegen only deals with any remaining COO new ops
(or something of the sort)

1311

indentation?

1354

getNextFuncName is a bit strange name for the ReaderRead call?

1379

This needs more documentation, since you have two tests ;-)

At compile time, you decide whether target needs ordering in the last level (and thus ordered COO).
So document that before L1324

Then, you have a run-time test on the notSorted boolean that comes out of getSparseTensorReaderRead(
Make that a bit more clear on L1325
(and yes, you have it at L1256 but good to repeat here.

bixia updated this revision to Diff 499535.Feb 22 2023, 9:04 AM
bixia marked 4 inline comments as done.

Address review comments.

mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
1323

I think it is dimRank. In line 1274, we use it to get the dimSizes of the tensor, for dynamic shape. In line 1305, we use it for the size of dim2lvl mapping.

1354

Renamed to getReadFuncName.

1379

Add comment.

bixia updated this revision to Diff 501163.Feb 28 2023, 8:38 AM

Rebase.

aartbik accepted this revision.Feb 28 2023, 11:50 AM
This revision is now accepted and ready to land.Feb 28 2023, 11:50 AM
bixia updated this revision to Diff 501268.Feb 28 2023, 12:53 PM

Fix codegen.mlir