This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] Factoring out SparseTensorFile class for readSparseTensorShape
ClosedPublic

Authored by wrengr on May 23 2022, 12:12 PM.

Details

Summary

The primary goal of this change is to define readSparseTensorShape. Whereas the SparseTensorFile class is merely introduced as a way to reduce code duplication along the way.

Depends On D126106

Diff Detail

Event Timeline

wrengr created this revision.May 23 2022, 12:12 PM
wrengr requested review of this revision.May 23 2022, 12:12 PM
bixia added inline comments.May 23 2022, 2:32 PM
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp
1112

s/Disallow/Disallows/

1116

Avoids or Tries to avoid

1176

Gets

1182

Gets

wrengr updated this revision to Diff 432362.May 26 2022, 12:58 PM
wrengr marked 4 inline comments as done.

Addressing nits. And identifying a problem for MSVC (to be fixed shortly).

wrengr updated this revision to Diff 432391.May 26 2022, 2:50 PM

First attempt to fix the MSVC build error.

wrengr updated this revision to Diff 432411.May 26 2022, 4:27 PM

Removing the fixme comment, since the changes in Diff 432391 successfully correct the MSVC build errors. (The Debian build failure appears to be unrelated to any change in this differential.)

aartbik accepted this revision.May 27 2022, 2:07 PM
aartbik added inline comments.
mlir/lib/ExecutionEngine/SparseTensorUtils.cpp
1138–1139

preferably with zero cost overhead, since that will be called for every nonzero :-)

1776

typoe: Received

This revision is now accepted and ready to land.May 27 2022, 2:07 PM
wrengr updated this revision to Diff 433168.May 31 2022, 12:59 PM
wrengr marked 2 inline comments as done.

Fixing typo

mlir/lib/ExecutionEngine/SparseTensorUtils.cpp
1138–1139

indeed :) All the obvious refactorings I considered weren't zero-cost (or leaked abstraction in the other direction), hence why the comment is here at all rather than just doing it :)