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>

src/reader/spirv/function.cc

@@ -4080,6 +4080,13 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) {
       // 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 {

src/reader/spirv/parser_impl_handle_test.cc

@@ -3549,7 +3549,9 @@ INSTANTIATE_TEST_SUITE_P(
 
         //
         // 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 @@ INSTANTIATE_TEST_SUITE_P(
           }
         }
       }
-    })"},
-        // 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 @@ TEST_P(SpvParserTest_ImageCoordsTest, MakeCoordinateOperandsForImageAccess) {
   )";
   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(
          {}},
     }));
 
+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