This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] cleanup ABI issues in C interface with memrefs
ClosedPublic

Authored by aartbik on Sep 21 2021, 8:37 PM.

Details

Summary

This change adds automatic wrapper functoins with emit_c_interface
to all methods in the sparse support library that deal with MEMREFs.
The wrappers will take care of passing MEMREFs by value internally
and by pointer externally, thereby avoiding ABI issues across platforms.

Diff Detail

Event Timeline

aartbik created this revision.Sep 21 2021, 8:37 PM
aartbik requested review of this revision.Sep 21 2021, 8:37 PM
mehdi_amini accepted this revision.Sep 21 2021, 8:52 PM

That was fast! Thanks :)

mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorConversion.cpp
104

(naming convention: camelCase)

180

(same below)

This revision is now accepted and ready to land.Sep 21 2021, 8:52 PM
aartbik updated this revision to Diff 374105.Sep 21 2021, 8:53 PM

fix clang tidy issue

aartbik updated this revision to Diff 374106.Sep 21 2021, 9:01 PM
aartbik marked an inline comment as done.

addressed Mehdi's comments

bondhugula added inline comments.
mlir/lib/ExecutionEngine/SparseUtils.cpp
114–116

Unrelated side question: shouldn't this file be named SparseTensorUtils.cpp?

aartbik marked an inline comment as done.Sep 21 2021, 9:19 PM
aartbik added inline comments.
mlir/lib/ExecutionEngine/SparseUtils.cpp
114–116

Yeah, I see this as a "sparse support library" but a "sparse tensor support library" would make sense too. Happy to rename (later). We will probably do some other cleanup too, such as adding a header.

aartbik updated this revision to Diff 374107.Sep 21 2021, 9:26 PM
aartbik marked an inline comment as done.

NOLINT on _mlir_ciface_XXX name

aartbik updated this revision to Diff 374108.Sep 21 2021, 9:35 PM

NOLINT on right line this time ;-)