This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] support parsing slices in sparse tensor encoding attribute
ClosedPublic

Authored by Peiming on Dec 27 2022, 5:43 PM.

Diff Detail

Event Timeline

Peiming created this revision.Dec 27 2022, 5:43 PM
Herald added a project: Restricted Project. · View Herald Transcript
Peiming requested review of this revision.Dec 27 2022, 5:43 PM

roundtrip and validation tests? also makes it easier to see what it will look like

mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
253

This needs a lot more documentation what the triplets mean per dimension ;-)

Peiming updated this revision to Diff 487635.Jan 9 2023, 6:19 PM

support parsing in dynamic slices.

Peiming retitled this revision from [mlir][sparse] support parsing constant slices in sparse tensor encoding attribute to [mlir][sparse] support parsing slices in sparse tensor encoding attribute.Jan 9 2023, 6:19 PM
bixia added inline comments.Jan 11 2023, 9:47 AM
mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
23

We normally end a comment with a period, see also SparseTensorOps.td.

29–32

Do we intend to add the description?

42

offset/size/stride to be consistent with ordering of the parameters in the code?

43

Would it be better if we use mlir::ShapedType::kDynamic instead?
Do you know why mlir::ShapedType::kDynamic was change from -1 to std::numeric_limits<int64_t>::min()?

mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp
65

Would it be better if we say "positive value or ? for" here?

Peiming updated this revision to Diff 488319.Jan 11 2023, 11:50 AM
Peiming marked 5 inline comments as done.

address comments

mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
29–32

No, Sorry that I forget to add the description here...

43

I do not know why it is changed, any negative value should be okay though?

mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp
65

SG

Peiming updated this revision to Diff 488327.Jan 11 2023, 12:09 PM

code simplification.

Peiming updated this revision to Diff 488330.Jan 11 2023, 12:13 PM

revert unintended changes.

bixia accepted this revision.Jan 11 2023, 4:05 PM
This revision is now accepted and ready to land.Jan 11 2023, 4:05 PM
Peiming updated this revision to Diff 488768.Jan 12 2023, 1:56 PM

fix typo.