This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] moving kInvalidId into "detail" namespace
ClosedPublic

Authored by wrengr on Mar 22 2023, 5:13 PM.

Details

Summary

In the next few commits I will be converting the various Merger identifier typedefs into newtypes; and once that's done, the kInvalidId constant will only be used internally and therefore does not need to be part of the public mlir::sparse_tensor namespace.

Depends On D146673

Diff Detail

Event Timeline

wrengr created this revision.Mar 22 2023, 5:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 22 2023, 5:13 PM
wrengr requested review of this revision.Mar 22 2023, 5:13 PM
aartbik accepted this revision.Mar 22 2023, 6:33 PM
aartbik added inline comments.
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1114

are you deliberately enforcing the ::mlir top level instead of relying on the using mlir::sparse_tensor with the shorter version of this?
(I assume yes, but just checking)

This revision is now accepted and ready to land.Mar 22 2023, 6:33 PM
wrengr added inline comments.Mar 22 2023, 6:41 PM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1114

yeah it's deliberate, just for paranoia's sake really. I'm pretty sure just using sparse_tensor::detail::kInvalidId would work too. But in the next few CLs this'll be replaced with if (!e.isValid()) so I'm not woo worried about the verbiage in the interim

wrengr updated this revision to Diff 507595.EditedMar 22 2023, 9:28 PM

arc diff $(git merge-base HEAD origin) --update D146674

This revision was automatically updated to reflect the committed changes.