This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] add support for direct prod/and/min/max reductions
ClosedPublic

Authored by aartbik on Jun 9 2023, 12:49 PM.

Details

Summary

We recently fixed a bug in "sparsifying" such reductions, since
it incorrectly changed this into reductions over stored elements
only , which only works for add/sub/or/xor. However, we still want
to be able to "sparsify" the reductions even in the general case,
and this is a first step by rewriting them into a custom reduction
that feeds in the implicit zeros. NOTE HOWEVER, that in the long run
we want to do this better and feed in any implicit zero only ONCE
for efficiency.

Diff Detail

Event Timeline

aartbik created this revision.Jun 9 2023, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2023, 12:49 PM
aartbik requested review of this revision.Jun 9 2023, 12:49 PM
aartbik updated this revision to Diff 530060.Jun 9 2023, 12:53 PM

removed unrelated file

wrengr added inline comments.Jun 9 2023, 12:56 PM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
393

Why "only useful" for min/max? Are you unfamiliar with tropical and Viterbi semirings? (https://en.wikipedia.org/wiki/Tropical_semiring, https://winterkoninkje.dreamwidth.org/80410.html) Granted the tropical semirings will want a different "zero" value

aartbik marked an inline comment as done.Jun 9 2023, 1:01 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
393

I merely meant that PROD/AND over sparse data seems a bit less likely since it would nullify quite often (so I see more use for custom prod over just the stored elements).
But having said that, I still recognize PROD/AND even here so we are fully functional, and we can have cases where e.g. many rows are fully dense, even in sparse situations.

If you prefer, I can remove the comment.

aartbik updated this revision to Diff 530065.Jun 9 2023, 1:04 PM
aartbik marked an inline comment as done.

addressed comment

wrengr added inline comments.Jun 9 2023, 1:05 PM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
421–425

You can actually pass a list of template parameters to isa, which has the desired disjunctive semantics

aartbik marked an inline comment as done.Jun 9 2023, 1:26 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorRewriting.cpp
421–425

Oh, that is so neat. Yeah, I have used that in other case, and here it makes a lot of sense!

aartbik updated this revision to Diff 530068.Jun 9 2023, 1:27 PM
aartbik marked an inline comment as done.

parameterized isa<> call

Peiming accepted this revision.Jun 12 2023, 9:26 AM
This revision is now accepted and ready to land.Jun 12 2023, 9:26 AM