Page MenuHomePhabricator

[mlir][sparse] implement singleton dimension level type
AcceptedPublic

Authored by aartbik on Fri, Sep 16, 6:28 PM.

Details

Summary

This is a first step towards fully implementing the new dimension
level types and properties, illustrating with a fully functional
sorted COO of any dimension. Note that the sparsification part is
pretty complete. The required parts in the runtime support library
have been kept to a minimum, to avoid huge conflicts with Wren's
ongoing refactoring. The missing parts will be filled in later.

Event Timeline

aartbik created this revision.Fri, Sep 16, 6:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Sep 16, 6:28 PM
aartbik requested review of this revision.Fri, Sep 16, 6:28 PM
Peiming added inline comments.Mon, Sep 19, 9:05 AM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1390–1391

How about having a merger.isSparseLevel utils?

Peiming added inline comments.Mon, Sep 19, 9:15 AM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1327–1328

What if you have lo = 0, hi = 1 here for singleton? (to allow the loop unroll opt kicking in later?)

1369

And you do not overwrite the value here. (the pidxs for singleton will become loop invariant, but it is okay because there is no loop actually).

aartbik marked 3 inline comments as done.Mon, Sep 19, 10:21 AM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1327–1328

Yeah that would be a good way to get the current loop unroller working.

But I also feel we would break the regularity of codegen in sparsification just for the sake of working around a limitation in unrolling. Besides, I have put one of my best people on the task of improving unrolling ;-)

1369

Note that the "loop" could co-iterates with another, and then it remains. I have a CHECK example for that ready in the next revision.

1390–1391

Yeah, good idea, I can also use that for the scan one.
(do you insist of having it in this revision?)

Peiming added inline comments.Mon, Sep 19, 11:28 AM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1221

I am wondering which one of the followings should we use?

(d0) -> (d0 + 1)
or
()[s0]->(s0 + 1)

aartbik marked 3 inline comments as done.Mon, Sep 19, 11:58 AM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1221

Waiting to see what feedback you get on https://reviews.llvm.org/D134204

Peiming added inline comments.Mon, Sep 19, 1:57 PM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1390–1391

No, not really.

Peiming accepted this revision.Mon, Sep 19, 2:32 PM
This revision is now accepted and ready to land.Mon, Sep 19, 2:32 PM
aartbik updated this revision to Diff 461419.Mon, Sep 19, 5:02 PM

rebased with Peiming's unrolling: single loop for COO now!

Holding off landing this revision until Wren's chain of refactoring lands, and I will rebase this then before submitting.

aartbik added inline comments.Tue, Sep 20, 12:54 PM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1221

Note that I reverted this again to addi over affine apply

aartbik updated this revision to Diff 461666.Tue, Sep 20, 12:59 PM

rebased with Peiming's recent change (affine apply -> addi)

aartbik updated this revision to Diff 462053.Wed, Sep 21, 5:40 PM

rebase with main

aartbik updated this revision to Diff 462059.Wed, Sep 21, 6:27 PM

rebased with main

aartbik updated this revision to Diff 462188.Thu, Sep 22, 8:58 AM

rebased with main