From 0073a1fa36f7c52ad3d58059cb5d5de8efa825ad Mon Sep 17 00:00:00 2001
From: Spencer Fricke <spencerfricke@gmail.com>
Date: Thu, 18 Aug 2022 00:37:05 +0900
Subject: [PATCH] spirv-val: Remove ImageWrite Texel todo (#4899)

---
 source/val/validate_image.cpp | 11 +----------
 test/val/val_image_test.cpp   | 24 ++++++++++++++++++++----
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/source/val/validate_image.cpp b/source/val/validate_image.cpp
index 2d5e2c7c5..1d9c877e9 100644
--- a/source/val/validate_image.cpp
+++ b/source/val/validate_image.cpp
@@ -1689,8 +1689,7 @@ spv_result_t ValidateImageWrite(ValidationState_t& _, const Instruction* inst) {
            << " components, but given only " << actual_coord_size;
   }
 
-  // TODO(atgoo@github.com) The spec doesn't explicitly say what the type
-  // of texel should be.
+  // because it needs to match with 'Sampled Type' the Texel can't be a boolean
   const uint32_t texel_type = _.GetOperandTypeId(inst, 2);
   if (!_.IsIntScalarOrVectorType(texel_type) &&
       !_.IsFloatScalarOrVectorType(texel_type)) {
@@ -1698,14 +1697,6 @@ spv_result_t ValidateImageWrite(ValidationState_t& _, const Instruction* inst) {
            << "Expected Texel to be int or float vector or scalar";
   }
 
-#if 0
-  // TODO: See above.
-  if (_.GetDimension(texel_type) != 4) {
-    return _.diag(SPV_ERROR_INVALID_DATA, inst)
-        << "Expected Texel to have 4 components";
-  }
-#endif
-
   if (_.GetIdOpcode(info.sampled_type) != SpvOpTypeVoid) {
     const uint32_t texel_component_type = _.GetComponentType(texel_type);
     if (texel_component_type != info.sampled_type) {
diff --git a/test/val/val_image_test.cpp b/test/val/val_image_test.cpp
index c4de60a25..df1402460 100644
--- a/test/val/val_image_test.cpp
+++ b/test/val/val_image_test.cpp
@@ -149,6 +149,7 @@ OpDecorate %input_flat_u32 Location 0
 %u32vec4 = OpTypeVector %u32 4
 %s32vec4 = OpTypeVector %s32 4
 %f32vec4 = OpTypeVector %f32 4
+%boolvec4 = OpTypeVector %bool 4
 
 %f32_0 = OpConstant %f32 0
 %f32_1 = OpConstant %f32 1
@@ -175,6 +176,8 @@ OpDecorate %input_flat_u32 Location 0
 %u64_0 = OpConstant %u64 0
 %u64_1 = OpConstant %u64 1
 
+%bool_t = OpConstantTrue %bool
+
 %u32vec2arr4 = OpTypeArray %u32vec2 %u32_4
 %u32vec2arr3 = OpTypeArray %u32vec2 %u32_3
 %u32arr4 = OpTypeArray %u32 %u32_4
@@ -217,6 +220,8 @@ OpDecorate %input_flat_u32 Location 0
 
 %f32vec4_0000 = OpConstantComposite %f32vec4 %f32_0 %f32_0 %f32_0 %f32_0
 
+%boolvec4_tttt = OpConstantComposite %boolvec4 %bool_t %bool_t %bool_t %bool_t
+
 %const_offsets = OpConstantComposite %u32vec2arr4 %u32vec2_01 %u32vec2_12 %u32vec2_01 %u32vec2_12
 %const_offsets3x2 = OpConstantComposite %u32vec2arr3 %u32vec2_01 %u32vec2_12 %u32vec2_01
 %const_offsets4xu = OpConstantComposite %u32arr4 %u32_0 %u32_0 %u32_0 %u32_0
@@ -1120,7 +1125,7 @@ TEST_F(ValidateImage, ImageTexelPointerImageNotResultTypePointer) {
 
   CompileSuccessfully(GenerateShaderCode(body).c_str());
   ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 145[%145] cannot be a "
+  EXPECT_THAT(getDiagnosticString(), HasSubstr("Operand 148[%148] cannot be a "
                                                "type"));
 }
 
@@ -3746,6 +3751,17 @@ OpImageWrite %img %u32_1 %u32vec4_0123
                         "but given only 1"));
 }
 
+TEST_F(ValidateImage, WriteTexelScalarSuccess) {
+  const std::string body = R"(
+%img = OpLoad %type_image_u32_2d_0002 %uniform_image_u32_2d_0002
+OpImageWrite %img %u32vec2_01 %u32_2
+)";
+
+  const std::string extra = "\nOpCapability StorageImageWriteWithoutFormat\n";
+  CompileSuccessfully(GenerateShaderCode(body, extra).c_str());
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
 TEST_F(ValidateImage, WriteTexelWrongType) {
   const std::string body = R"(
 %img = OpLoad %type_image_u32_2d_0002 %uniform_image_u32_2d_0002
@@ -3759,17 +3775,17 @@ OpImageWrite %img %u32vec2_01 %img
               HasSubstr("Expected Texel to be int or float vector or scalar"));
 }
 
-TEST_F(ValidateImage, DISABLED_WriteTexelNotVector4) {
+TEST_F(ValidateImage, WriteTexelNonNumericalType) {
   const std::string body = R"(
 %img = OpLoad %type_image_u32_2d_0002 %uniform_image_u32_2d_0002
-OpImageWrite %img %u32vec2_01 %u32vec3_012
+OpImageWrite %img %u32vec2_01 %boolvec4_tttt
 )";
 
   const std::string extra = "\nOpCapability StorageImageWriteWithoutFormat\n";
   CompileSuccessfully(GenerateShaderCode(body, extra).c_str());
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Expected Texel to have 4 components"));
+              HasSubstr("Expected Texel to be int or float vector or scalar"));
 }
 
 TEST_F(ValidateImage, WriteTexelWrongComponentType) {