This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] Cleaning up the dim/lvl distinction in SparseTensorConversion
ClosedPublic

Authored by wrengr on Dec 1 2022, 7:25 PM.

Details

Summary

This change cleans up the conversion pass re the "dim"-vs-"lvl" and "sizes"-vs-"shape" distinctions of the runtime. A quick synopsis includes:

  • Adds new SparseTensorStorageBase::getDimSize method, with sparseDimSize wrapper in SparseTensorRuntime.h, and genDimSizeCall generator in SparseTensorConversion.cpp
  • Changes genLvlSizeCall to perform no logic, just generate the function call.
  • Adds createOrFold{Dim,Lvl}Call functions to handle the logic of replacing gen{Dim,Lvl}SizeCall with constants whenever possible. The createOrFoldDimCall function replaces the old sizeFromPtrAtDim.
  • Adds {get,fill}DimSizes functions for iterating createOrFoldDimCall across the whole type. These functions replace the old sizesFromPtr.
  • Adds {get,fill}DimShape functions for lowering a ShapedType into constants. These functions replace the old sizesFromType.
  • Changes the DimOp rewrite to do the right thing.
  • Changes the ExpandOp rewrite to compute the proper expansion size.

Depends On D138365

Diff Detail

Event Timeline

wrengr created this revision.Dec 1 2022, 7:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2022, 7:25 PM
wrengr requested review of this revision.Dec 1 2022, 7:25 PM
aartbik accepted this revision.Dec 5 2022, 4:48 PM
aartbik added inline comments.
mlir/test/Dialect/SparseTensor/conversion.mlir
364

any reason for dropping the signature?
(and good catch on the brittle match using c1 ;-)

This revision is now accepted and ready to land.Dec 5 2022, 4:48 PM
wrengr added inline comments.Dec 5 2022, 4:54 PM
mlir/test/Dialect/SparseTensor/conversion.mlir
364

Just hoping to avoid brittleness is all. The signature doesn't add any information here, and the different test files seem to vary in whether they include the signatures or not, so I went with the one that seemed more common. I can add it back in if you'd prefer