Page MenuHomePhabricator

[mlir][sparse] add create-sparse-deallocs options to match the create-deallocs in BufferizationOption.
ClosedPublic

Authored by Peiming on Mar 27 2023, 3:57 PM.

Diff Detail

Event Timeline

Peiming created this revision.Mar 27 2023, 3:57 PM
Herald added a project: Restricted Project. · View Herald Transcript
Peiming requested review of this revision.Mar 27 2023, 3:57 PM
wrengr added inline comments.Mar 27 2023, 4:00 PM
mlir/include/mlir/Dialect/SparseTensor/Pipelines/Passes.h
78–81

Should document how this option interacts with enableRuntimeLibrary. That is, I think this is only used/needed for the codegen path rather than the runtime path?

aartbik added inline comments.Mar 27 2023, 4:01 PM
mlir/include/mlir/Dialect/SparseTensor/Pipelines/Passes.h
78

by the sparse compiler

mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.td
224

by the sparse compiler

mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
1521

how about not adding rule when disabled?

wrengr added inline comments.Mar 27 2023, 4:02 PM
mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h
185–188

Any particular reason you're putting createSparseDeallocs there, rather than at the end?

aartbik added inline comments.Mar 27 2023, 4:05 PM
mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorCodegen.cpp
1521

my mistake, we still erase...

Peiming marked 4 inline comments as done.Mar 27 2023, 4:10 PM
Peiming added inline comments.
mlir/include/mlir/Dialect/SparseTensor/Pipelines/Passes.h
78–81

Yeah, you are right.

mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h
185–188

by alphabet order (c < e)?

Peiming updated this revision to Diff 508831.Mar 27 2023, 4:11 PM
Peiming marked an inline comment as done.

address comments.

aartbik accepted this revision.Mar 27 2023, 4:15 PM
aartbik added inline comments.
mlir/test/Dialect/SparseTensor/codegen_sparse_dealloc.mlir
17

user-requested

18

can you add a

CHECK-NO-DEALLOC-LABEL: @sparse_convert_permuted

and

CHECK-DEALLOC-LABEL: @sparse_convert_permuted

It clearly works without, but it feels a bit cleaner to avoid matching output elsewhere if this every changes

This revision is now accepted and ready to land.Mar 27 2023, 4:15 PM
wrengr added inline comments.Mar 27 2023, 4:16 PM
mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h
185–188

fair enough :)

Peiming updated this revision to Diff 508836.Mar 27 2023, 4:17 PM

address comments.

This revision was landed with ongoing or failed builds.Mar 27 2023, 4:18 PM
This revision was automatically updated to reflect the committed changes.
wrengr added inline comments.Mar 27 2023, 4:20 PM
mlir/include/mlir/Dialect/SparseTensor/Pipelines/Passes.h
78–83

I would rephrase this to "Specifies whether to deallocate the temporary sparse buffers created by the sparse compiler when enable-runtime-library=false. For compatibility with core bufferization passes. See also..."