[mlir][spirv] Verify SampledImageType Dim #159397
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-spirv Author: Igor Wodiany (IgWod-IMG) ChangesThis 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:
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>
|
kuhar
approved these changes
Sep 17, 2025
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.
IgWod-IMG
force-pushed
the
img_sampled-image-verification
branch
from
40031e4 to
9582289
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.