This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] sparse tensor type encoding migration (new home, new builders)
ClosedPublic

Authored by aartbik on Apr 30 2021, 3:09 PM.

Details

Summary

(1) migrates the encoding from TensorDialect into the new SparseTensorDialect
(2) replaces dictionary-based storage and builders with struct-like data

Diff Detail

Event Timeline

aartbik created this revision.Apr 30 2021, 3:09 PM
aartbik requested review of this revision.Apr 30 2021, 3:09 PM
aartbik updated this revision to Diff 342072.Apr 30 2021, 3:43 PM

fixed macro name convention

Looks great!

One thing still missing is to enforce the invariant on the SparseTensorEncodingAttr with a verify() method.
The verifyEncoding is really about "is this attribute compatible with a given tensor", but the SparseTensorEncodingAttr also has it own invariants as a standalone construct. For example:
The dimLevelType size should match the dimOrdering size, and the dimOrdering should be a permutation (i.e. only a subset of the AffineMap are possible).

rriddle added inline comments.Apr 30 2021, 3:47 PM
mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
17

Can you wrap this at 80 characters?

aartbik marked an inline comment as done.Apr 30 2021, 4:18 PM

One thing still missing is to enforce the invariant on the SparseTensorEncodingAttr with a verify() method.

Yes, that is a good idea. Right now we verify through parsing and IR's verification, but this will add a level of verification on the pure constructor.
Added...

aartbik updated this revision to Diff 342096.Apr 30 2021, 5:31 PM

add an explicit verifier that is shared between other
verification paths, but also used by e.g. constructor

mehdi_amini accepted this revision.Apr 30 2021, 5:32 PM

Thanks!

This revision is now accepted and ready to land.Apr 30 2021, 5:32 PM
rriddle added inline comments.Apr 30 2021, 5:34 PM
mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp
114–118
147

Drop the llvm:: and mlir:: here, and throughout this commit.

aartbik marked 2 inline comments as done.Apr 30 2021, 5:58 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp
114–118

Oh, that is brilliant. Thanks for pointing this out. Much nicer!

aartbik updated this revision to Diff 342100.Apr 30 2021, 6:23 PM
aartbik marked an inline comment as done.

use nifty MLIR stuff

mlir/test/Dialect/SparseTensor/roundtrip_encoding.mlir