This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] add int64 storage type to sparse tensor runtime support library
ClosedPublic

Authored by aartbik on Jul 14 2021, 2:00 PM.

Details

Summary

This format was missing from the support library. Although there are some
subtleties reading in an external format for int64 as double, there is no
good reason to omit support for this data type form the support library.

Diff Detail

Event Timeline

aartbik created this revision.Jul 14 2021, 2:00 PM
aartbik requested review of this revision.Jul 14 2021, 2:00 PM

Just one question, otherwise looks good!

mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorConversion.cpp
180–186

Is it possible to use the PrimaryTypeEnum here, or use some kind of enum?

aartbik marked an inline comment as done.Jul 15 2021, 10:37 AM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorConversion.cpp
180–186

Yes, that is a good suggestion. AFAIK, we are not allowed to create shared dependences between runtime and compiler (as in including the same header file). But I agree this hardcoding is a bit too error prone, so I at least created a shadow enum with a comment in this file.

aartbik updated this revision to Diff 359058.Jul 15 2021, 11:12 AM
aartbik marked an inline comment as done.

added note on value consistency with support lib

aartbik updated this revision to Diff 359061.Jul 15 2021, 11:14 AM

oops, removed debug code

gussmith23 accepted this revision.Jul 15 2021, 12:05 PM
This revision is now accepted and ready to land.Jul 15 2021, 12:05 PM
This revision was landed with ongoing or failed builds.Jul 15 2021, 12:14 PM
This revision was automatically updated to reflect the committed changes.