Skip to content

Conversation

@CoTinker
Copy link
Contributor

This PR adds null check for spirvVectorType in VectorLoadOpConverter to prevent a crash. Fixes #149956.

CoTinker added 2 commits July 22, 2025 14:26
This PR adds null check for `spirvVectorType` in VectorLoadOpConverter to prevent a crash.
@llvmbot
Copy link
Member

llvmbot commented Jul 22, 2025

@llvm/pr-subscribers-mlir

Author: Longsheng Mou (CoTinker)

Changes

This PR adds null check for spirvVectorType in VectorLoadOpConverter to prevent a crash. Fixes #149956.


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

2 Files Affected:

  • (modified) mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp (+3)
  • (modified) mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir (+12)
diff --git a/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp b/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
index 750ce85049409..06b19335f2aed 100644
--- a/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
+++ b/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
@@ -793,6 +793,9 @@ struct VectorLoadOpConverter final
     // Use the converted vector type instead of original (single element vector
     // would get converted to scalar).
     auto spirvVectorType = typeConverter.convertType(vectorType);
+    if (!spirvVectorType)
+      return rewriter.notifyMatchFailure(loadOp, "unsupported vector type");
+
     auto vectorPtrType = spirv::PointerType::get(spirvVectorType, storageClass);
 
     // For single element vectors, we don't need to bitcast the access chain to
diff --git a/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir b/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir
index 44941f0d0ac5d..f43a41a0af2f4 100644
--- a/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir
+++ b/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir
@@ -1161,3 +1161,15 @@ func.func @vector_store_2d(%arg0 : memref<4x4xf32, #spirv.storage_class<StorageB
 }
 
 } // end module
+
+// -----
+
+// Ensure the case without module attributes not crash.
+
+// CHECK-LABEL: @vector_load
+//       CHECK:   vector.load
+func.func @vector_load(%arg0 : memref<4xf32, #spirv.storage_class<StorageBuffer>>) -> vector<4xf32> {
+  %idx = arith.constant 0 : index
+  %0 = vector.load %arg0[%idx] : memref<4xf32, #spirv.storage_class<StorageBuffer>>, vector<4xf32>
+  return %0: vector<4xf32>
+}

@llvmbot
Copy link
Member

llvmbot commented Jul 22, 2025

@llvm/pr-subscribers-mlir-spirv

Author: Longsheng Mou (CoTinker)

Changes

This PR adds null check for spirvVectorType in VectorLoadOpConverter to prevent a crash. Fixes #149956.


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

2 Files Affected:

  • (modified) mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp (+3)
  • (modified) mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir (+12)
diff --git a/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp b/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
index 750ce85049409..06b19335f2aed 100644
--- a/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
+++ b/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp
@@ -793,6 +793,9 @@ struct VectorLoadOpConverter final
     // Use the converted vector type instead of original (single element vector
     // would get converted to scalar).
     auto spirvVectorType = typeConverter.convertType(vectorType);
+    if (!spirvVectorType)
+      return rewriter.notifyMatchFailure(loadOp, "unsupported vector type");
+
     auto vectorPtrType = spirv::PointerType::get(spirvVectorType, storageClass);
 
     // For single element vectors, we don't need to bitcast the access chain to
diff --git a/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir b/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir
index 44941f0d0ac5d..f43a41a0af2f4 100644
--- a/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir
+++ b/mlir/test/Conversion/VectorToSPIRV/vector-to-spirv.mlir
@@ -1161,3 +1161,15 @@ func.func @vector_store_2d(%arg0 : memref<4x4xf32, #spirv.storage_class<StorageB
 }
 
 } // end module
+
+// -----
+
+// Ensure the case without module attributes not crash.
+
+// CHECK-LABEL: @vector_load
+//       CHECK:   vector.load
+func.func @vector_load(%arg0 : memref<4xf32, #spirv.storage_class<StorageBuffer>>) -> vector<4xf32> {
+  %idx = arith.constant 0 : index
+  %0 = vector.load %arg0[%idx] : memref<4xf32, #spirv.storage_class<StorageBuffer>>, vector<4xf32>
+  return %0: vector<4xf32>
+}

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Contributor

@mshahneo mshahneo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks a lot.

@CoTinker CoTinker merged commit 020856e into llvm:main Jul 22, 2025
12 checks passed
@CoTinker CoTinker deleted the vector_load branch July 22, 2025 11:26
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
…9964)

This PR adds null check for `spirvVectorType` in VectorLoadOpConverter
to prevent a crash. Fixes llvm#149956.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MLIR] Segmentation Fault when converting VectorLoadOp

4 participants