[Mlir-commits] [mlir] [mlir][spirv] Verify SampledImageType Dim (PR #159397)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Sep 17 09:46:14 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-spirv
Author: Igor Wodiany (IgWod-IMG)
<details>
<summary>Changes</summary>
This patches adds check for: "It [ImageType] must not have a Dim of SubpassData. Additionally, starting with version 1.6, it must not have a Dim of Buffer." as defined in "3.3.6. Type-Declaration Instructions" of SPIR-V spec.
---
Full diff: https://github.com/llvm/llvm-project/pull/159397.diff
5 Files Affected:
- (modified) mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp (+9-1)
- (modified) mlir/lib/Dialect/SPIRV/IR/SPIRVTypes.cpp (+8-1)
- (modified) mlir/test/Dialect/SPIRV/IR/image-ops.mlir (+9-24)
- (modified) mlir/test/Dialect/SPIRV/IR/types.mlir (+10)
- (modified) mlir/test/Target/SPIRV/sampled-image.mlir (+2-2)
``````````diff
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp
index 44c86bc8777e4..c8efdf0094223 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVDialect.cpp
@@ -251,13 +251,21 @@ static Type parseAndVerifySampledImageType(SPIRVDialect const &dialect,
if (parser.parseType(type))
return Type();
- if (!llvm::isa<ImageType>(type)) {
+ auto imageType = dyn_cast<ImageType>(type);
+ if (!imageType) {
parser.emitError(typeLoc,
"sampled image must be composed using image type, got ")
<< type;
return Type();
}
+ if (llvm::is_contained({Dim::SubpassData, Dim::Buffer}, imageType.getDim())) {
+ parser.emitError(
+ typeLoc, "sampled image Dim must not be SubpassData or Buffer, got ")
+ << stringifyDim(imageType.getDim());
+ return Type();
+ }
+
return type;
}
diff --git a/mlir/lib/Dialect/SPIRV/IR/SPIRVTypes.cpp b/mlir/lib/Dialect/SPIRV/IR/SPIRVTypes.cpp
index 369b95374189d..d890dac96b118 100644
--- a/mlir/lib/Dialect/SPIRV/IR/SPIRVTypes.cpp
+++ b/mlir/lib/Dialect/SPIRV/IR/SPIRVTypes.cpp
@@ -805,9 +805,16 @@ Type SampledImageType::getImageType() const { return getImpl()->imageType; }
LogicalResult
SampledImageType::verifyInvariants(function_ref<InFlightDiagnostic()> emitError,
Type imageType) {
- if (!llvm::isa<ImageType>(imageType))
+ auto image = dyn_cast<ImageType>(imageType);
+ if (!image)
return emitError() << "expected image type";
+ // As per SPIR-V spec: "It [ImageType] must not have a Dim of SubpassData.
+ // Additionally, starting with version 1.6, it must not have a Dim of Buffer.
+ // ("3.3.6. Type-Declaration Instructions")
+ if (llvm::is_contained({Dim::SubpassData, Dim::Buffer}, image.getDim()))
+ return emitError() << "Dim must not be SubpassData or Buffer";
+
return success();
}
diff --git a/mlir/test/Dialect/SPIRV/IR/image-ops.mlir b/mlir/test/Dialect/SPIRV/IR/image-ops.mlir
index 320a8fa360a5f..7369d719ca53d 100644
--- a/mlir/test/Dialect/SPIRV/IR/image-ops.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/image-ops.mlir
@@ -192,6 +192,9 @@ func.func @image_write_texel_type_mismatch(%arg0 : !spirv.image<f32, Dim2D, NoDe
// spirv.ImageSampleExplicitLod
//===----------------------------------------------------------------------===//
+// No need to have a negative test for Dim being Buffer as this is already handled
+// by SampledImageType logic.
+
func.func @sample_explicit_lod(%arg0 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, %arg1 : vector<2xf32>, %arg2 : f32) -> () {
// CHECK: {{%.*}} = spirv.ImageSampleExplicitLod {{%.*}}, {{%.*}} ["Lod"], {{%.*}} : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<2xf32>, f32 -> vector<4xf32>
%0 = spirv.ImageSampleExplicitLod %arg0, %arg1 ["Lod"], %arg2 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<2xf32>, f32 -> vector<4xf32>
@@ -200,14 +203,6 @@ func.func @sample_explicit_lod(%arg0 : !spirv.sampled_image<!spirv.image<f32, Di
// -----
-func.func @sample_explicit_lod_buffer_dim(%arg0 : !spirv.sampled_image<!spirv.image<f32, Buffer, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, %arg1 : vector<2xf32>, %arg2 : f32) -> () {
- // expected-error @+1 {{the Dim operand of the underlying image must not be Buffer}}
- %0 = spirv.ImageSampleExplicitLod %arg0, %arg1 ["Lod"], %arg2 : !spirv.sampled_image<!spirv.image<f32, Buffer, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<2xf32>, f32 -> vector<4xf32>
- spirv.Return
-}
-
-// -----
-
func.func @sample_explicit_lod_multi_sampled(%arg0 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, MultiSampled, NeedSampler, Rgba8>>, %arg1 : vector<2xf32>, %arg2 : f32) -> () {
// expected-error @+1 {{the MS operand of the underlying image type must be SingleSampled}}
%0 = spirv.ImageSampleExplicitLod %arg0, %arg1 ["Lod"], %arg2 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, MultiSampled, NeedSampler, Rgba8>>, vector<2xf32>, f32 -> vector<4xf32>
@@ -236,6 +231,9 @@ func.func @sample_explicit_lod_no_lod(%arg0 : !spirv.sampled_image<!spirv.image<
// spirv.ImageSampleImplicitLod
//===----------------------------------------------------------------------===//
+// No need to have a negative test for Dim being Buffer as this is already handled
+// by SampledImageType logic.
+
func.func @sample_implicit_lod(%arg0 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, %arg1 : vector<2xf32>) -> () {
// CHECK: {{%.*}} = spirv.ImageSampleImplicitLod {{%.*}}, {{%.*}} : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<2xf32> -> vector<4xf32>
%0 = spirv.ImageSampleImplicitLod %arg0, %arg1 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<2xf32> -> vector<4xf32>
@@ -244,14 +242,6 @@ func.func @sample_implicit_lod(%arg0 : !spirv.sampled_image<!spirv.image<f32, Di
// -----
-func.func @sample_implicit_lod_buffer(%arg0 : !spirv.sampled_image<!spirv.image<f32, Buffer, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, %arg1 : vector<2xf32>) -> () {
- // expected-error @+1 {{the Dim operand of the underlying image must not be Buffer}}
- %0 = spirv.ImageSampleImplicitLod %arg0, %arg1 : !spirv.sampled_image<!spirv.image<f32, Buffer, NoDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<2xf32> -> vector<4xf32>
- spirv.Return
-}
-
-// -----
-
func.func @sample_implicit_lod_multi_sampled(%arg0 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, MultiSampled, NeedSampler, Rgba8>>, %arg1 : vector<2xf32>) -> () {
// expected-error @+1 {{the MS operand of the underlying image type must be SingleSampled}}
%0 = spirv.ImageSampleImplicitLod %arg0, %arg1 : !spirv.sampled_image<!spirv.image<f32, Dim2D, NoDepth, NonArrayed, MultiSampled, NeedSampler, Rgba8>>, vector<2xf32> -> vector<4xf32>
@@ -272,6 +262,9 @@ func.func @sample_implicit_lod_wrong_result(%arg0 : !spirv.sampled_image<!spirv.
// spirv.ImageSampleProjDrefImplicitLod
//===----------------------------------------------------------------------===//
+// No need to have a negative test for Dim being Buffer as this is already handled
+// by SampledImageType logic.
+
func.func @sample_implicit_proj_dref(%arg0 : !spirv.sampled_image<!spirv.image<f32, Dim2D, IsDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, %arg1 : vector<4xf32>, %arg2 : f32) -> () {
// CHECK: {{%.*}} = spirv.ImageSampleProjDrefImplicitLod {{%.*}}, {{%.*}}, {{%.*}} : !spirv.sampled_image<!spirv.image<f32, Dim2D, IsDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<4xf32>, f32 -> f32
%0 = spirv.ImageSampleProjDrefImplicitLod %arg0, %arg1, %arg2 : !spirv.sampled_image<!spirv.image<f32, Dim2D, IsDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<4xf32>, f32 -> f32
@@ -280,14 +273,6 @@ func.func @sample_implicit_proj_dref(%arg0 : !spirv.sampled_image<!spirv.image<f
// -----
-func.func @sample_implicit_proj_dref_buffer_dim(%arg0 : !spirv.sampled_image<!spirv.image<f32, Buffer, IsDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, %arg1 : vector<4xf32>, %arg2 : f32) -> () {
- // expected-error @+1 {{the Dim operand of the underlying image must not be Buffer}}
- %0 = spirv.ImageSampleProjDrefImplicitLod %arg0, %arg1, %arg2 : !spirv.sampled_image<!spirv.image<f32, Buffer, IsDepth, NonArrayed, SingleSampled, NeedSampler, Rgba8>>, vector<4xf32>, f32 -> f32
- spirv.Return
-}
-
-// -----
-
func.func @sample_implicit_proj_dref_multi_sampled(%arg0 : !spirv.sampled_image<!spirv.image<f32, Dim2D, IsDepth, NonArrayed, MultiSampled, NeedSampler, Rgba8>>, %arg1 : vector<4xf32>, %arg2 : f32) -> () {
// expected-error @+1 {{the MS operand of the underlying image type must be SingleSampled}}
%0 = spirv.ImageSampleProjDrefImplicitLod %arg0, %arg1, %arg2 : !spirv.sampled_image<!spirv.image<f32, Dim2D, IsDepth, NonArrayed, MultiSampled, NeedSampler, Rgba8>>, vector<4xf32>, f32 -> f32
diff --git a/mlir/test/Dialect/SPIRV/IR/types.mlir b/mlir/test/Dialect/SPIRV/IR/types.mlir
index 6d321afebf7f8..f350e56255983 100644
--- a/mlir/test/Dialect/SPIRV/IR/types.mlir
+++ b/mlir/test/Dialect/SPIRV/IR/types.mlir
@@ -238,6 +238,16 @@ func.func private @samped_image_type_invaid_type(!spirv.sampled_image<f32>) -> (
// -----
+// expected-error @+1 {{sampled image Dim must not be SubpassData or Buffer, got Buffer}}
+func.func private @sampled_image_type(!spirv.sampled_image<!spirv.image<f32, Buffer, NoDepth, NonArrayed, SingleSampled, NoSampler, Unknown>>) -> ()
+
+// -----
+
+// expected-error @+1 {{sampled image Dim must not be SubpassData or Buffer, got SubpassData}}
+func.func private @sampled_image_type(!spirv.sampled_image<!spirv.image<f32, SubpassData, NoDepth, NonArrayed, SingleSampled, NoSampler, Unknown>>) -> ()
+
+// -----
+
//===----------------------------------------------------------------------===//
// StructType
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Target/SPIRV/sampled-image.mlir b/mlir/test/Target/SPIRV/sampled-image.mlir
index 694862d943534..ddf18d6eee16a 100644
--- a/mlir/test/Target/SPIRV/sampled-image.mlir
+++ b/mlir/test/Target/SPIRV/sampled-image.mlir
@@ -4,8 +4,8 @@ spirv.module Logical GLSL450 requires #spirv.vce<v1.0, [Shader], []> {
// CHECK: !spirv.ptr<!spirv.sampled_image<!spirv.image<f32, Dim1D, NoDepth, NonArrayed, SingleSampled, NoSampler, Unknown>>, UniformConstant>
spirv.GlobalVariable @var0 bind(0, 1) : !spirv.ptr<!spirv.sampled_image<!spirv.image<f32, Dim1D, NoDepth, NonArrayed, SingleSampled, NoSampler, Unknown>>, UniformConstant>
- // CHECK: !spirv.ptr<!spirv.sampled_image<!spirv.image<si32, SubpassData, DepthUnknown, Arrayed, MultiSampled, NoSampler, Unknown>>, UniformConstant>
- spirv.GlobalVariable @var1 bind(0, 0) : !spirv.ptr<!spirv.sampled_image<!spirv.image<si32, SubpassData, DepthUnknown, Arrayed, MultiSampled, NoSampler, Unknown>>, UniformConstant>
+ // CHECK: !spirv.ptr<!spirv.sampled_image<!spirv.image<si32, Cube, DepthUnknown, Arrayed, MultiSampled, NeedSampler, Unknown>>, UniformConstant>
+ spirv.GlobalVariable @var1 bind(0, 0) : !spirv.ptr<!spirv.sampled_image<!spirv.image<si32, Cube, DepthUnknown, Arrayed, MultiSampled, NeedSampler, Unknown>>, UniformConstant>
// CHECK: !spirv.ptr<!spirv.sampled_image<!spirv.image<i32, Rect, DepthUnknown, Arrayed, MultiSampled, NeedSampler, R8ui>>, UniformConstant>
spirv.GlobalVariable @var2 bind(0, 0) : !spirv.ptr<!spirv.sampled_image<!spirv.image<i32, Rect, DepthUnknown, Arrayed, MultiSampled, NeedSampler, R8ui>>, UniformConstant>
``````````
</details>
https://github.com/llvm/llvm-project/pull/159397
More information about the Mlir-commits
mailing list