This is an archive of the discontinued LLVM Phabricator instance.

Upstream MLIR PyTACO implementation.
ClosedPublic

Authored by bixia on Jan 13 2022, 4:27 PM.

Details

Summary

Add TACO tests to test/Integration/Dialect/SparseTensor/taco. Add the MLIR
PyTACO implementation as tools under the directory.

Diff Detail

Event Timeline

bixia created this revision.Jan 13 2022, 4:27 PM
bixia requested review of this revision.Jan 13 2022, 4:27 PM
mehdi_amini added inline comments.Jan 13 2022, 4:47 PM
mlir/test/Integration/Dialect/SparseTensor/taco/tools/mlir_pytaco.py
151

Could this be implemented as a registered C++ pipeline instead of listing all the passes here?

aartbik added inline comments.Jan 13 2022, 5:31 PM
mlir/test/Integration/Dialect/SparseTensor/taco/tools/mlir_pytaco.py
151

Yeah, agreed, this is very related to https://github.com/llvm/llvm-project/issues/51751 where we express the need to avoid proliferating these veyr specific passes all over the place.

aartbik added inline comments.Jan 18 2022, 9:19 AM
mlir/test/Integration/Dialect/SparseTensor/taco/MTTKRP_test.py
12 ↗(On Diff #399828)

can you make this a line break comment, as in

This PyTACO part is taken from the TACO open-source project.

and then close it at line 41 with

just so that the part that we used from TACO jumps more out

17 ↗(On Diff #399828)

maybe spell out "formats"

mlir/test/Integration/Dialect/SparseTensor/taco/SpMV_test.py
12 ↗(On Diff #399828)

same request

mlir/test/Integration/Dialect/SparseTensor/taco/tools/mlir_pytaco.py
5

empty line after copyright header

81

np.float16

151

Bixia, for now, I would suggest to add a TODO and link to bug

aartbik added inline comments.Jan 18 2022, 9:20 AM
mlir/test/Integration/Dialect/SparseTensor/taco/MTTKRP_test.py
12 ↗(On Diff #399828)

Ah, now I see what went wrong last time, my suggestion is being formatted by phacricator.
I really want something as

#### This PyTACO part is taken from the TACO open-source project. ####

and then end it as

#######################################
mehdi_amini added inline comments.Jan 18 2022, 5:54 PM
mlir/test/Integration/Dialect/SparseTensor/taco/tools/mlir_pytaco.py
151

This seems like a trivial thing to do isn't it? It probably won't take much longer that the TODO and filing the bug :)

bixia updated this revision to Diff 401412.Jan 19 2022, 2:40 PM
bixia marked 9 inline comments as done.

Address review comments.
Rename test files so that module names start with lower cases to confirm to python style guide.

bixia added inline comments.Jan 19 2022, 2:41 PM
mlir/test/Integration/Dialect/SparseTensor/taco/tools/mlir_pytaco.py
151

Replaced this with a TODO as the ticket already has a owner and I can't just take over it.

mehdi_amini accepted this revision.Jan 20 2022, 3:43 PM
mehdi_amini added inline comments.
mlir/test/Integration/Dialect/SparseTensor/taco/tools/mlir_pytaco.py
151

Alright, LG, thanks :)

This revision is now accepted and ready to land.Jan 20 2022, 3:43 PM
aartbik accepted this revision.Jan 20 2022, 3:54 PM

Well done Bixia!

This revision was automatically updated to reflect the committed changes.