spirv-reader: reject sampling operations on non-float textures
Fixed: tint:350
Bug: tint:109
Change-Id: I754ff4ade03dabb70a1eb5706a65c48a0a7d35a8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35581
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc
index 386fdf1..82d9ec5 100644
--- a/src/reader/spirv/function.cc
+++ b/src/reader/spirv/function.cc
@@ -4080,6 +4080,13 @@
// or vice versa. Perform a bitcast.
value = create<ast::BitcastExpression>(Source{}, result_type, call_expr);
}
+ if (!expected_component_type->Is<ast::type::F32>() &&
+ IsSampledImageAccess(inst.opcode())) {
+ // WGSL permits sampled image access only on float textures.
+ // Reject this case in the SPIR-V reader, at least until SPIR-V validation
+ // catches up with this rule and can reject it earlier in the workflow.
+ return Fail() << "sampled image must have float component type";
+ }
EmitConstDefOrWriteToHoistedVar(inst, {result_type, value});
} else {
diff --git a/src/reader/spirv/parser_impl_handle_test.cc b/src/reader/spirv/parser_impl_handle_test.cc
index e0ee403..83055a6 100644
--- a/src/reader/spirv/parser_impl_handle_test.cc
+++ b/src/reader/spirv/parser_impl_handle_test.cc
@@ -3549,7 +3549,9 @@
//
// Sampling operations, using OpImageSampleImplicitLod as an example.
- //
+ // WGSL sampling operations only work on textures with a float sampled
+ // component. So we can only test the float -> float (non-conversion)
+ // case.
// OpImageSampleImplicitLod requires no conversion, float -> v4float
{"%float 2D 0 0 0 1 Unknown",
@@ -3589,156 +3591,6 @@
}
}
}
- })"},
- // OpImageSampleImplicitLod requires no conversion, uint -> v4uint
- {"%uint 2D 0 0 0 1 Unknown",
- "%99 = OpImageSampleImplicitLod %v4uint %sampled_image %vf12",
- R"(
- Variable{
- Decorations{
- SetDecoration{0}
- BindingDecoration{0}
- }
- x_10
- uniform_constant
- __sampler_sampler
- }
- Variable{
- Decorations{
- SetDecoration{2}
- BindingDecoration{1}
- }
- x_20
- uniform_constant
- __sampled_texture_2d__u32
- })",
- R"(VariableDeclStatement{
- VariableConst{
- x_99
- none
- __vec_4__u32
- {
- Call[not set]{
- Identifier[not set]{textureSample}
- (
- Identifier[not set]{x_20}
- Identifier[not set]{x_10}
- Identifier[not set]{vf12}
- )
- }
- }
- }
- })"},
- // OpImageSampleImplicitLod requires conversion, uint -> v4int
- {"%uint 2D 0 0 0 1 Unknown",
- "%99 = OpImageSampleImplicitLod %v4int %sampled_image %vf12",
- R"(
- Variable{
- Decorations{
- SetDecoration{0}
- BindingDecoration{0}
- }
- x_10
- uniform_constant
- __sampler_sampler
- }
- Variable{
- Decorations{
- SetDecoration{2}
- BindingDecoration{1}
- }
- x_20
- uniform_constant
- __sampled_texture_2d__u32
- })",
- R"(VariableDeclStatement{
- VariableConst{
- x_99
- none
- __vec_4__i32
- {
- Bitcast[not set]<__vec_4__i32>{
- Call[not set]{
- Identifier[not set]{textureSample}
- (
- Identifier[not set]{x_20}
- Identifier[not set]{x_10}
- Identifier[not set]{vf12}
- )
- }
- }
- }
- }
- })"},
- // OpImageSampleImplicitLod requires no conversion, int -> v4int
- {"%int 2D 0 0 0 1 Unknown",
- "%99 = OpImageSampleImplicitLod %v4int %sampled_image %vf12",
- R"(
- Variable{
- Decorations{
- SetDecoration{0}
- BindingDecoration{0}
- }
- x_10
- uniform_constant
- __sampler_sampler
- }
- Variable{
- Decorations{
- SetDecoration{2}
- BindingDecoration{1}
- }
- x_20
- uniform_constant
- __sampled_texture_2d__i32
- })",
- R"(VariableDeclStatement{
- VariableConst{
- x_99
- none
- __vec_4__i32
- {
- Call[not set]{
- Identifier[not set]{textureSample}
- (
- Identifier[not set]{x_20}
- Identifier[not set]{x_10}
- Identifier[not set]{vf12}
- )
- }
- }
- }
- })"},
- // OpImageSampleImplicitLod requires conversion, int -> v4uint
- {"%int 2D 0 0 0 1 Unknown",
- "%99 = OpImageSampleImplicitLod %v4uint %sampled_image %vf12",
- R"(Variable{
- Decorations{
- SetDecoration{2}
- BindingDecoration{1}
- }
- x_20
- uniform_constant
- __sampled_texture_2d__i32
- })",
- R"(VariableDeclStatement{
- VariableConst{
- x_99
- none
- __vec_4__u32
- {
- Bitcast[not set]<__vec_4__u32>{
- Call[not set]{
- Identifier[not set]{textureSample}
- (
- Identifier[not set]{x_20}
- Identifier[not set]{x_10}
- Identifier[not set]{vf12}
- )
- }
- }
- }
- }
})"}}));
struct ImageCoordsCase {
@@ -3837,7 +3689,7 @@
)";
auto p = parser(test::Assemble(assembly));
if (!p->BuildAndParseInternalModule()) {
- EXPECT_THAT(p->error(), Eq(GetParam().expected_error));
+ EXPECT_THAT(p->error(), Eq(GetParam().expected_error)) << assembly;
} else {
EXPECT_TRUE(p->error().empty()) << p->error();
FunctionEmitter fe(p.get(), *spirv_function(p.get(), 100));
@@ -4486,6 +4338,53 @@
{}},
}));
+INSTANTIATE_TEST_SUITE_P(
+ SampleNonFloatTexture_IsError,
+ SpvParserTest_ImageCoordsTest,
+ ::testing::ValuesIn(std::vector<ImageCoordsCase>{
+ // ImageSampleImplicitLod
+ {"%uint 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleImplicitLod %v4uint %sampled_image %vf12",
+ "sampled image must have float component type",
+ {}},
+ {"%int 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleImplicitLod %v4int %sampled_image %vf12",
+ "sampled image must have float component type",
+ {}},
+ // ImageSampleExplicitLod
+ {"%uint 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleExplicitLod %v4uint %sampled_image %vf12 "
+ "Lod %f1",
+ "sampled image must have float component type",
+ {}},
+ {"%int 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleExplicitLod %v4int %sampled_image %vf12 "
+ "Lod %f1",
+ "sampled image must have float component type",
+ {}},
+ // ImageSampleDrefImplicitLod
+ {"%uint 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleDrefImplicitLod %v4uint %sampled_image %vf12 "
+ "%f1",
+ "sampled image must have float component type",
+ {}},
+ {"%int 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleDrefImplicitLod %v4int %sampled_image %vf12 "
+ "%f1",
+ "sampled image must have float component type",
+ {}},
+ // ImageSampleDrefExplicitLod
+ {"%uint 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleDrefExplicitLod %v4uint %sampled_image %vf12 "
+ "%f1 Lod %f1",
+ "sampled image must have float component type",
+ {}},
+ {"%int 2D 0 0 0 1 Unknown",
+ "%result = OpImageSampleDrefExplicitLod %v4int %sampled_image %vf12 "
+ "%f1 Lod %f1",
+ "sampled image must have float component type",
+ {}}}));
+
} // namespace
} // namespace spirv
} // namespace reader
