This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] Add rewriting rules for concatente.
AbandonedPublic

Authored by Peiming on Sep 21 2022, 2:42 PM.

Event Timeline

Peiming created this revision.Sep 21 2022, 2:42 PM
Herald added a project: Restricted Project. · View Herald Transcript
Peiming requested review of this revision.Sep 21 2022, 2:42 PM
Peiming updated this revision to Diff 462013.Sep 21 2022, 2:47 PM

add test cases

Peiming added inline comments.Sep 21 2022, 2:49 PM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
595–601

This is currently copied form https://reviews.llvm.org/D134096, will be resolved after the parents revision is merged.

mlir/test/Dialect/SparseTensor/sparse_concat_codegen.mlir
38

This (and following pointers and indices) will (hopefully) be eliminated after lowering.

Peiming added inline comments.Sep 21 2022, 2:55 PM
mlir/test/Dialect/SparseTensor/sparse_concat_codegen.mlir
37

Note that the hasInserter is not set here

aartbik added inline comments.Sep 22 2022, 9:10 AM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
113

Unordered

121

I think the n-2 level needs to be not-unique as wel (so NuNo ;-) and then the last one unique again (so No)

127

how about 0? default native

303

why not else if?

We have a general rule on no if after return, but I think this case does not apply when there is a new if

322

What is still missing here? Can we assert on that?

331

unordered

335

Nice! Very progressive lowering approach. Nicolas will be proud of you!

352

period at end

mlir/test/Dialect/SparseTensor/sparse_concat_codegen.mlir
3

empty line before decl, also, name this DCSR

Peiming added inline comments.Sep 22 2022, 9:26 AM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
303

Okay, I just followed what clang-tidy suggested me (no else on return), I can revert it back.

Peiming updated this revision to Diff 462215.Sep 22 2022, 9:45 AM
Peiming marked 8 inline comments as done.

address comments.

Peiming added inline comments.Sep 22 2022, 9:47 AM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
121

Yeah, make sense...

Do you think n-1 level need to be ordered? Considering all the input tensors is ordered right now?

303

per offline discussion, we will keep the change.

mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
211

Any reason that we iterate over output tensor as well here?

Peiming marked an inline comment as not done.Sep 23 2022, 10:56 AM
Peiming abandoned this revision.Sep 27 2022, 3:31 PM

Per offline discussion, this will be implemented using foreach operator instead.