Skip to content

[mlir][SparseTensor] Fix type conversion rule #140350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

matthias-springer
Copy link
Member

A type conversion rule cannot make any assumptions about the number of pre-existing types in the results vector.

This commit fixes a failed assertion in a SparseTensor type conversion rule. This is only reproducible when type conversion caching is deactivated. There's no way to do this at the moment. This commit is in preparation of adding context-aware type conversions, which will deactivate type caching in such cases.

Depends on #140347.

@llvmbot
Copy link
Member

llvmbot commented May 17, 2025

@llvm/pr-subscribers-mlir-sparse

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

A type conversion rule cannot make any assumptions about the number of pre-existing types in the results vector.

This commit fixes a failed assertion in a SparseTensor type conversion rule. This is only reproducible when type conversion caching is deactivated. There's no way to do this at the moment. This commit is in preparation of adding context-aware type conversions, which will deactivate type caching in such cases.

Depends on #140347.


Full diff: https://github.com/llvm/llvm-project/pull/140350.diff

1 Files Affected:

  • (modified) mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorDescriptor.cpp (+5-4)
diff --git a/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorDescriptor.cpp b/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorDescriptor.cpp
index 8bbb2cac5efdf..79602a22dc1fe 100644
--- a/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorDescriptor.cpp
+++ b/mlir/lib/Dialect/SparseTensor/Transforms/Utils/SparseTensorDescriptor.cpp
@@ -38,12 +38,13 @@ convertSparseTensorType(RankedTensorType rtp, SmallVectorImpl<Type> &fields) {
   if (!stt.hasEncoding())
     return std::nullopt;
 
+  unsigned numFields = fields.size();
   foreachFieldAndTypeInSparseTensor(
       stt,
-      [&fields](Type fieldType, FieldIndex fieldIdx,
-                SparseTensorFieldKind /*fieldKind*/, Level /*lvl*/,
-                LevelType /*lt*/) -> bool {
-        assert(fieldIdx == fields.size());
+      [&](Type fieldType, FieldIndex fieldIdx,
+          SparseTensorFieldKind /*fieldKind*/, Level /*lvl*/,
+          LevelType /*lt*/) -> bool {
+        assert(numFields + fieldIdx == fields.size());
         fields.push_back(fieldType);
         return true;
       });

@PeimingLiu
Copy link
Member

Thx!

Base automatically changed from users/matthias-springer/convert_type_assert to main May 18, 2025 00:05
@matthias-springer matthias-springer force-pushed the users/matthias-springer/fix_type_converter_sparse branch from f14a088 to 5a46d6d Compare May 18, 2025 00:08
@matthias-springer matthias-springer merged commit 3360a23 into main May 18, 2025
7 of 11 checks passed
@matthias-springer matthias-springer deleted the users/matthias-springer/fix_type_converter_sparse branch May 18, 2025 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:sparse Sparse compiler in MLIR mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants