This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] template the memory resident coordinate scheme storage
ClosedPublic

Authored by aartbik on Jul 28 2021, 1:49 PM.

Details

Summary

Rationale:
External file formats always store the values as doubles, so this was
hard coded in the memory resident COO scheme used to pass data into the
final sparse storage scheme during setup. However, with alternative methods
on the horizon of setting up these temporary COO schemes, it is time to
properly template this data structure.

Diff Detail

Event Timeline

aartbik created this revision.Jul 28 2021, 1:49 PM
aartbik requested review of this revision.Jul 28 2021, 1:49 PM
gussmith23 accepted this revision.Jul 30 2021, 9:38 AM

I left a comment re: naming, but everything looks good otherwise!

mlir/lib/ExecutionEngine/SparseUtils.cpp
154

Hm, the fact that the naming is inconsistent here (that is, this typename is V in both SparseTensor and Element) is potentially confusing. Maybe you could use V instead of W here, and rename the other V? Alternatively, you could rename the V used in SparseTensor and Element?

This revision is now accepted and ready to land.Jul 30 2021, 9:38 AM
aartbik marked an inline comment as done.Jul 30 2021, 9:44 AM
aartbik added inline comments.
mlir/lib/ExecutionEngine/SparseUtils.cpp
154

Interesting that you bring this up, since this was bothering me too. I like the consistent V naming (since it is really the value type parameter) and the friction here is purely due to external formats always using double (normally you would simply pass the single V to "both" for a consistent naming). Renaming either would creep into all other places.

How about using Vm and Ve (for the V for memory and extermal)? Or V1 and V2?

aartbik updated this revision to Diff 363132.Jul 30 2021, 10:08 AM
aartbik marked an inline comment as done.

renamed W (the "second" V) to Ve