validation: validate return type of OpImageRead (#4072)

Vulkan: must be 4-element vector WebGPU: must be 4-element vector OpenCL: - must be scalar float for depth image - must be 4-element vector otherwise
2024-10-18 11:10:05 +00:00 · 2020-12-15 12:00:59 -05:00 · 2020-12-15 12:00:59 -05:00 · ad898cb949
commit ad898cb949
parent 305caff2eb
3 changed files with 350 additions and 16 deletions
--- a/source/val/validate_image.cpp
+++ b/source/val/validate_image.cpp
@ -1444,14 +1444,16 @@ spv_result_t ValidateImageRead(ValidationState_t& _, const Instruction* inst) {
           << " to be int or float scalar or vector type";
  }

-#if 0
-  // TODO(atgoo@github.com) Disabled until the spec is clarified.
-  if (_.GetDimension(actual_result_type) != 4) {
-    return _.diag(SPV_ERROR_INVALID_DATA, inst)
-           << "Expected " << GetActualResultTypeStr(opcode)
-           << " to have 4 components";
-  }
-#endif
+  const auto target_env = _.context()->target_env;
+  // Vulkan and WebGPU require the result to be a 4-element int or float
+  // vector.
+  if (spvIsVulkanEnv(target_env) || spvIsWebGPUEnv(target_env)) {
+    if (_.GetDimension(actual_result_type) != 4) {
+      return _.diag(SPV_ERROR_INVALID_DATA, inst)
+             << "Expected " << GetActualResultTypeStr(opcode)
+             << " to have 4 components";
+    }
+  }  // Check OpenCL below, after we get the image info.

  const uint32_t image_type = _.GetOperandTypeId(inst, 2);
  if (_.GetIdOpcode(image_type) != SpvOpTypeImage) {
@ -1465,6 +1467,29 @@ spv_result_t ValidateImageRead(ValidationState_t& _, const Instruction* inst) {
           << "Corrupt image type definition";
  }

+  if (spvIsOpenCLEnv(target_env)) {
+    // In OpenCL, a read from a depth image returns a scalar float. In other
+    // cases, the result is always a 4-element vector.
+    // https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_Env.html#_data_format_for_reading_and_writing_images
+    // https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#image-read-and-write-functions
+    // The builtins for reading depth images are:
+    //   float read_imagef(aQual image2d_depth_t image, int2 coord)
+    //   float read_imagef(aQual image2d_array_depth_t image, int4 coord)
+    if (info.depth) {
+      if (!_.IsFloatScalarType(actual_result_type)) {
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << "Expected " << GetActualResultTypeStr(opcode)
+               << " from a depth image read to result in a scalar float value";
+      }
+    } else {
+      if (_.GetDimension(actual_result_type) != 4) {
+        return _.diag(SPV_ERROR_INVALID_DATA, inst)
+               << "Expected " << GetActualResultTypeStr(opcode)
+               << " to have 4 components";
+      }
+    }
+  }
+
  if (info.dim == SpvDimSubpassData) {
    if (opcode == SpvOpImageSparseRead) {
      return _.diag(SPV_ERROR_INVALID_DATA, inst)
--- a/test/val/val_image_test.cpp
+++ b/test/val/val_image_test.cpp
@ -463,6 +463,13 @@ std::string GetWebGPUShaderHeader() {
  %func = OpTypeFunction %void
  %f32 = OpTypeFloat 32
  %u32 = OpTypeInt 32 0
+  %f32vec2 = OpTypeVector %f32 2
+  %f32vec3 = OpTypeVector %f32 3
+  %f32vec4 = OpTypeVector %f32 4
+  %u32vec2 = OpTypeVector %u32 2
+  %u32vec3 = OpTypeVector %u32 3
+  %u32vec4 = OpTypeVector %u32 4
+  %u32vec2null = OpConstantNull %u32vec2
  )";
 }

@ -3131,16 +3138,61 @@ TEST_F(ValidateImage, DISABLED_ReadWrongResultType) {
              HasSubstr("Expected Result Type to be int or float vector type"));
 }

-// TODO(atgoo@github.com) Disabled until the spec is clarified.
-TEST_F(ValidateImage, DISABLED_ReadWrongNumComponentsResultType) {
+TEST_F(ValidateImage, ReadScalarResultType_Universal) {
  const std::string body = R"(
 %img = OpLoad %type_image_u32_2d_0002 %uniform_image_u32_2d_0002
-%res1 = OpImageRead %f32vec3 %img %u32vec2_01
+%res1 = OpImageRead %u32 %img %u32vec2_01
 )";

  const std::string extra = "\nOpCapability StorageImageReadWithoutFormat\n";
  CompileSuccessfully(GenerateShaderCode(body, extra).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_0));
+  EXPECT_THAT(getDiagnosticString(), Eq(""));
+}
+
+TEST_F(ValidateImage, ReadUnusualNumComponentsResultType_Universal) {
+  const std::string body = R"(
+%img = OpLoad %type_image_u32_2d_0002 %uniform_image_u32_2d_0002
+%res1 = OpImageRead %u32vec3 %img %u32vec2_01
+)";
+
+  const std::string extra = "\nOpCapability StorageImageReadWithoutFormat\n";
+  CompileSuccessfully(GenerateShaderCode(body, extra).c_str());
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_0));
+  EXPECT_THAT(getDiagnosticString(), Eq(""));
+}
+
+TEST_F(ValidateImage, ReadWrongNumComponentsResultType_Vulkan) {
+  const std::string body = R"(
+%img = OpLoad %type_image_u32_2d_0002 %uniform_image_u32_2d_0002
+%res1 = OpImageRead %u32vec3 %img %u32vec2_01
+)";
+
+  const std::string extra = "\nOpCapability StorageImageReadWithoutFormat\n";
+  CompileSuccessfully(
+      GenerateShaderCode(body, extra, "Fragment", "", SPV_ENV_VULKAN_1_0)
+          .c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Result Type to have 4 components"));
+}
+
+TEST_F(ValidateImage, ReadWrongNumComponentsResultType_WebGPU) {
+  const std::string code = GetWebGPUShaderHeader() + R"(
+%img_type = OpTypeImage %f32 2D 0 0 0 2 Unknown
+%ptr_type = OpTypePointer UniformConstant %img_type
+%var = OpVariable %ptr_type UniformConstant
+%main = OpFunction %void None %func
+%entry = OpLabel
+%img = OpLoad %img_type %var
+%res1 = OpImageRead %u32vec3 %img %u32vec2null
+OpReturn
+OpFunctionEnd
+)";
+
+  CompileSuccessfully(code.c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_WEBGPU_0))
+      << getDiagnosticString();
  EXPECT_THAT(getDiagnosticString(),
              HasSubstr("Expected Result Type to have 4 components"));
 }
--- a/test/val/val_opencl_test.cpp
+++ b/test/val/val_opencl_test.cpp
@ -23,6 +23,7 @@ namespace spvtools {
 namespace val {
 namespace {

+using testing::Eq;
 using testing::HasSubstr;

 using ValidateOpenCL = spvtest::ValidateBase<bool>;
@ -224,6 +225,264 @@ TEST_F(ValidateOpenCL, ImageReadWithConstOffsetBad) {
          "\n  %call = OpImageRead %v4uint %img %coord ConstOffset %coord\n"));
 }

+TEST_F(ValidateOpenCL, ImageRead_NonDepthScalarFloatResult_Bad) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+      %float = OpTypeFloat 32
+          %3 = OpTypeImage %void 2D 0 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %float %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Result Type to have 4 components"));
+}
+
+TEST_F(ValidateOpenCL, ImageRead_NonDepthScalarIntResult_Bad) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+      %float = OpTypeFloat 32
+          %3 = OpTypeImage %void 2D 0 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %uint %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Result Type to have 4 components"));
+}
+
+TEST_F(ValidateOpenCL, ImageRead_NonDepthVector3FloatResult_Bad) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+      %float = OpTypeFloat 32
+    %v3float = OpTypeVector %float 3
+          %3 = OpTypeImage %void 2D 0 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %v3float %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Result Type to have 4 components"));
+}
+
+TEST_F(ValidateOpenCL, ImageRead_NonDepthVector4FloatResult_Ok) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+      %float = OpTypeFloat 32
+    %v4float = OpTypeVector %float 4
+          %3 = OpTypeImage %void 2D 0 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %v4float %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(), Eq(""));
+}
+
+TEST_F(ValidateOpenCL, ImageRead_NonDepthVector4IntResult_Ok) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+     %v4uint = OpTypeVector %uint 4
+          %3 = OpTypeImage %void 2D 0 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %v4uint %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(), Eq(""));
+}
+
+TEST_F(ValidateOpenCL, ImageRead_DepthScalarFloatResult_Ok) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+      %float = OpTypeFloat 32
+          %3 = OpTypeImage %void 2D 1 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %float %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(), Eq(""));
+}
+
+TEST_F(ValidateOpenCL, ImageRead_DepthScalarIntResult_Bad) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+      %float = OpTypeFloat 32
+          %3 = OpTypeImage %void 2D 1 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %uint %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Result Type from a depth image "
+                        "read to result in a scalar float value"));
+}
+
+TEST_F(ValidateOpenCL, ImageRead_DepthVectorFloatResult_Bad) {
+  std::string spirv = R"(
+               OpCapability Addresses
+               OpCapability Kernel
+               OpCapability ImageBasic
+               OpMemoryModel Physical64 OpenCL
+               OpEntryPoint Kernel %5 "image_kernel"
+               OpName %img "img"
+               OpName %coord "coord"
+               OpName %call "call"
+       %uint = OpTypeInt 32 0
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
+       %void = OpTypeVoid
+      %float = OpTypeFloat 32
+    %v4float = OpTypeVector %float 4
+          %3 = OpTypeImage %void 2D 1 0 0 0 Unknown ReadOnly
+          %4 = OpTypeFunction %void %3
+          %5 = OpFunction %void None %4
+        %img = OpFunctionParameter %3
+      %entry = OpLabel
+       %call = OpImageRead %v4float %img %coord
+               OpReturn
+               OpFunctionEnd
+)";
+
+  CompileSuccessfully(spirv);
+
+  EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_OPENCL_1_2));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Result Type from a depth image "
+                        "read to result in a scalar float value"));
+}
+
 TEST_F(ValidateOpenCL, ImageSampleExplicitLodWithConstOffsetBad) {
  std::string spirv = R"(
               OpCapability Addresses
@ -236,18 +495,16 @@ TEST_F(ValidateOpenCL, ImageSampleExplicitLodWithConstOffsetBad) {
               OpName %coord "coord"
               OpName %call "call"
       %uint = OpTypeInt 32 0
-     %uint_7 = OpConstant %uint 7
-     %uint_3 = OpConstant %uint 3
+     %v2uint = OpTypeVector %uint 2
+      %coord = OpConstantNull %v2uint
       %void = OpTypeVoid
          %3 = OpTypeImage %void 2D 0 0 0 0 Unknown ReadOnly
          %4 = OpTypeFunction %void %3
          %8 = OpTypeSampler
         %10 = OpTypeSampledImage %3
     %v4uint = OpTypeVector %uint 4
-     %v2uint = OpTypeVector %uint 2
      %float = OpTypeFloat 32
          %9 = OpConstantSampler %8 None 0 Nearest
-      %coord = OpConstantComposite %v2uint %uint_7 %uint_3
    %float_0 = OpConstant %float 0
          %5 = OpFunction %void None %4
          %6 = OpFunctionParameter %3