This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] generalize reduction support in sparse compiler
ClosedPublic

Authored by aartbik on Sep 21 2021, 3:15 PM.

Details

Summary

Now not just SUM, but also PRODUCT, AND, OR, XOR. The reductions
MIN and MAX are still to be done (also depends on recognizing
these operations in cmp-select constructs).

Diff Detail

Event Timeline

aartbik created this revision.Sep 21 2021, 3:15 PM
aartbik requested review of this revision.Sep 21 2021, 3:15 PM
aartbik edited the summary of this revision. (Show Details)
bixia accepted this revision.Sep 22 2021, 11:32 AM
bixia added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
316

This shouldn't be needed. Right?

367–370

It will also work if we move kAnd to share with kProduct, and kOr to share with kAdd and retire this case, right?
Which approach is better?

372

Similar to the above, this is not needed.

This revision is now accepted and ready to land.Sep 22 2021, 11:32 AM
aartbik marked 3 inline comments as done.Sep 22 2021, 11:36 AM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
316

All cases are indeed covered, but some sanitizer/linters like having this anyway (at least at one point, Mehdi, is this still the case?)

367–370

For And, you will need "-1", not "1" to have all bits set. kOr could indeed share the kAdd.
Note, however, that this case will also be used in the future for MIN/MAX reductions ;-)

bixia added inline comments.Sep 22 2021, 11:36 AM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
367–370

I think I was wrong with kAnd, it will need -1|...-1|r, not 1...1|r

aartbik updated this revision to Diff 374308.Sep 22 2021, 11:37 AM
aartbik marked 2 inline comments as done.

rebased with main

This revision was automatically updated to reflect the committed changes.
aartbik marked an inline comment as done.