This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] added codegen for dimop, pointers, indices, values
ClosedPublic

Authored by aartbik on Sep 1 2022, 12:44 PM.

Details

Summary

Demonstrates how sparse tensor type -> tuple -> getter
will eventually yield actual code on the memrefs directly

Diff Detail

Event Timeline

aartbik created this revision.Sep 1 2022, 12:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 1 2022, 12:44 PM
aartbik requested review of this revision.Sep 1 2022, 12:44 PM
aartbik retitled this revision from [mlir][sparse] added codegen for dimop, pointers, indices, values Demonstrates how sparse tensor type -> tuple -> getter will eventually yield actual code on the memrefs directly to [mlir][sparse] added codegen for dimop, pointers, indices, values.Sep 1 2022, 12:45 PM
aartbik edited the summary of this revision. (Show Details)
aartbik updated this revision to Diff 457387.Sep 1 2022, 1:52 PM

minor cleanup of the tuple util

aartbik updated this revision to Diff 457388.Sep 1 2022, 1:55 PM

edit in name

Peiming added inline comments.Sep 1 2022, 3:40 PM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
252

Why don't need an index conversion (toStored) here? The toPointersOp also take an already converted index as the input?

Peiming added inline comments.Sep 1 2022, 3:42 PM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
174

Value index can simply be determined by querying the length of tuple?

aartbik marked an inline comment as done.Sep 1 2022, 3:48 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
174

Yes, doh! Fixed ;-)

252

Correct. DimOp is user facing, so it applies to original dim order.
The sparse index/pointer/values refer to the actual storage order.

Peiming added inline comments.Sep 1 2022, 3:51 PM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
168

So pointer idx are also already converted? (That's why you do not accumulate ptrIdx here)

Also would it be more clear to have to function for indice/pointers?

168

Sorry, two functions for indices/pointers separately.

aartbik marked an inline comment as done.Sep 1 2022, 3:58 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
168

oh, I see. Perhaps, but this keeps the logic in one place.

Also, perhaps we want to simply always use 2 fields per dimension, so we don't need to scan and can determine it more quickly later. But I don't like the wasted fields. Perhaps we should set up some meta data on the side, so that this method becomes a simple lookup. Not sure yet what is best....

aartbik updated this revision to Diff 457435.Sep 1 2022, 3:59 PM

values are always last

aartbik updated this revision to Diff 457436.Sep 1 2022, 4:02 PM

rebased with main

Peiming accepted this revision.Sep 1 2022, 4:30 PM
This revision is now accepted and ready to land.Sep 1 2022, 4:30 PM
This revision was landed with ongoing or failed builds.Sep 1 2022, 4:36 PM
This revision was automatically updated to reflect the committed changes.