Page MenuHomePhabricator

[mlir][sparse] Enhancing sparse=>sparse conversion.
ClosedPublic

Authored by wrengr on Mar 18 2022, 8:08 PM.

Diff Detail

Event Timeline

wrengr created this revision.Mar 18 2022, 8:08 PM
wrengr requested review of this revision.Mar 18 2022, 8:09 PM
wrengr updated this revision to Diff 417401.Mar 22 2022, 2:15 PM

rebase, and fixing build error

wrengr updated this revision to Diff 417436.Mar 22 2022, 4:47 PM

redoing the diff

wrengr updated this revision to Diff 418670.Mar 28 2022, 11:59 AM

re-renaming the old appendPointer to appendCurrentPointer (since I've realized the name "finalizeSegment" is better used elsewhere).

wrengr updated this revision to Diff 419892.Apr 1 2022, 4:58 PM

Lots of reorganization, subsequent to all the recent changes in the parent revisions.

wrengr edited the summary of this revision. (Show Details)Apr 4 2022, 5:21 PM
wrengr updated this revision to Diff 421055.Apr 6 2022, 5:44 PM

Rebasing for D123166. Also removing a bunch of inline keywords, per MLIR style-guide.

wrengr updated this revision to Diff 428755.May 11 2022, 1:25 PM

Rebasing for the latest version of D122060 (2022-05-11T13:09)

This also introduces a minor change to the enumerator design. Namely, due to the splitting of SparseTensorEnumeratorBase<V> vs SparseTensorEnumerator<P,I,V>, I've had to add the new SparseTensorStorageBase::newEnumerator methods to allow constructing objects of the abstract base type without reintroducing the "<P,I> vs <Q,J> problem" for direct conversion.

Arg! It looks like when I installed the github version of Arcanist to avoid the https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1838310.html bug with Debian's version, the new version isn't automatically applying the formatting script :( So please bear with the overly-long lines for now

wrengr updated this revision to Diff 428771.May 11 2022, 1:59 PM

(attempting to get the clang-formatter to run again)

wrengr updated this revision to Diff 428772.May 11 2022, 2:02 PM

(Previous attempt didn't work, so fixing some things manually)

wrengr updated this revision to Diff 428798.May 11 2022, 4:05 PM

Rerunning git-clang-format

aartbik accepted this revision.May 16 2022, 1:16 PM
aartbik added inline comments.
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp
569

Yeah! All your abstraction work is paying off!

605

shall we make this an explicit null_ptr for clarity
(I am fine either way but perhaps making it explicit reads clearer)?

943

Fine for this revision, but would it make sense to move this into its own file in the longer run?
Just to split the storage vs. supporting classes a bit more. Probably applies to some of the other classes too by now.

1426

yeah, direct! Sweet!

mlir/test/Dialect/SparseTensor/conversion.mlir
5

period at end

mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_conversion_sparse2sparse.mlir
2

period at end of comment

This revision is now accepted and ready to land.May 16 2022, 1:16 PM
wrengr updated this revision to Diff 429856.May 16 2022, 2:33 PM
wrengr marked 2 inline comments as done.

Rebasing for D125596

This revision was automatically updated to reflect the committed changes.