validation: tighter validation of multisampled images (#4059)

* validation: tighter validation of multisampled images

- if MS=1, then Sample image operand is required
- Sampling operations are not permitted when MS=1

Fixes #4057, #4058

* Fail early for multisampled image for sampling, dref, gather

2 files changed, 278 insertions, 173 deletions

diff --git a/source/val/validate_image.cpp b/source/val/validate_image.cpp
index 0a0eeee8..1961a74d 100644
--- a/source/val/validate_image.cpp
+++ b/source/val/validate_image.cpp
@@ -16,8 +16,6 @@
 
 // Validates correctness of image instructions.
 
-#include "source/val/validate.h"
-
 #include <string>
 
 #include "source/diagnostic.h"
@@ -25,6 +23,7 @@
 #include "source/spirv_target_env.h"
 #include "source/util/bitutils.h"
 #include "source/val/instruction.h"
+#include "source/val/validate.h"
 #include "source/val/validate_scopes.h"
 #include "source/val/validation_state.h"
 
@@ -234,9 +233,10 @@ uint32_t GetMinCoordSize(SpvOp opcode, const ImageTypeInfo& info) {
 }
 
 // Checks ImageOperand bitfield and respective operands.
+// word_index is the index of the first word after the image-operand mask word.
 spv_result_t ValidateImageOperands(ValidationState_t& _,
                                    const Instruction* inst,
-                                   const ImageTypeInfo& info, uint32_t mask,
+                                   const ImageTypeInfo& info,
                                    uint32_t word_index) {
   static const bool kAllImageOperandsHandled = CheckAllImageOperandsHandled();
   (void)kAllImageOperandsHandled;
@@ -244,24 +244,43 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
   const SpvOp opcode = inst->opcode();
   const size_t num_words = inst->words().size();
 
-  // NonPrivate, Volatile, SignExtend, ZeroExtend take no operand words.
-  const uint32_t mask_bits_having_operands =
-      mask & ~uint32_t(SpvImageOperandsNonPrivateTexelKHRMask |
-                       SpvImageOperandsVolatileTexelKHRMask |
-                       SpvImageOperandsSignExtendMask |
-                       SpvImageOperandsZeroExtendMask);
-  size_t expected_num_image_operand_words =
-      spvtools::utils::CountSetBits(mask_bits_having_operands);
-  if (mask & SpvImageOperandsGradMask) {
-    // Grad uses two words.
-    ++expected_num_image_operand_words;
-  }
+  const bool have_explicit_mask = (word_index - 1 < num_words);
+  const uint32_t mask = have_explicit_mask ? inst->word(word_index - 1) : 0u;
+
+  if (have_explicit_mask) {
+    // NonPrivate, Volatile, SignExtend, ZeroExtend take no operand words.
+    const uint32_t mask_bits_having_operands =
+        mask & ~uint32_t(SpvImageOperandsNonPrivateTexelKHRMask |
+                         SpvImageOperandsVolatileTexelKHRMask |
+                         SpvImageOperandsSignExtendMask |
+                         SpvImageOperandsZeroExtendMask);
+    size_t expected_num_image_operand_words =
+        spvtools::utils::CountSetBits(mask_bits_having_operands);
+    if (mask & SpvImageOperandsGradMask) {
+      // Grad uses two words.
+      ++expected_num_image_operand_words;
+    }
 
-  if (expected_num_image_operand_words != num_words - word_index) {
+    if (expected_num_image_operand_words != num_words - word_index) {
+      return _.diag(SPV_ERROR_INVALID_DATA, inst)
+             << "Number of image operand ids doesn't correspond to the bit "
+                "mask";
+    }
+  } else if (num_words != word_index - 1) {
     return _.diag(SPV_ERROR_INVALID_DATA, inst)
            << "Number of image operand ids doesn't correspond to the bit mask";
   }
 
+  if (info.multisampled & (0 == (mask & SpvImageOperandsSampleMask))) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "Image Operand Sample is required for operation on "
+              "multi-sampled image";
+  }
+
+  // After this point, only set bits in the image operands mask can cause
+  // the module to be invalid.
+  if (mask == 0) return SPV_SUCCESS;
+
   if (spvtools::utils::CountSetBits(
           mask & (SpvImageOperandsOffsetMask | SpvImageOperandsConstOffsetMask |
                   SpvImageOperandsConstOffsetsMask)) > 1) {
@@ -296,10 +315,7 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
                 "or Cube";
     }
 
-    if (info.multisampled != 0) {
-      return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << "Image Operand Bias requires 'MS' parameter to be 0";
-    }
+    // Multisampled is already checked.
   }
 
   if (mask & SpvImageOperandsLodMask) {
@@ -338,10 +354,7 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
                 "or Cube";
     }
 
-    if (info.multisampled != 0) {
-      return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << "Image Operand Lod requires 'MS' parameter to be 0";
-    }
+    // Multisampled is already checked.
   }
 
   if (mask & SpvImageOperandsGradMask) {
@@ -374,10 +387,7 @@ spv_result_t ValidateImageOperands(ValidationState_t& _,
              << " components, but given " << dy_size;
     }
 
-    if (info.multisampled != 0) {
-      return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << "Image Operand Grad requires 'MS' parameter to be 0";
-    }
+    // Multisampled is already checked.
   }
 
   if (mask & SpvImageOperandsConstOffsetMask) {
@@ -613,12 +623,12 @@ spv_result_t ValidateImageCommon(ValidationState_t& _, const Instruction* inst,
 
     if (info.multisampled != 0) {
       return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << "Image Image 'MS' parameter to be 0";
+             << "Expected Image 'MS' parameter to be 0";
     }
 
     if (info.arrayed != 0) {
       return _.diag(SPV_ERROR_INVALID_DATA, inst)
-             << "Image Image 'arrayed' parameter to be 0";
+             << "Expected Image 'arrayed' parameter to be 0";
     }
   }
 
@@ -1122,6 +1132,14 @@ spv_result_t ValidateImageLod(ValidationState_t& _, const Instruction* inst) {
 
   if (spv_result_t result = ValidateImageCommon(_, inst, info)) return result;
 
+  if (info.multisampled) {
+    // When using image operands, the Sample image operand is required if and
+    // only if the image is multisampled (MS=1). The Sample image operand is
+    // only allowed for fetch, read, and write.
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "Sampling operation is invalid for multisample image";
+  }
+
   if (_.GetIdOpcode(info.sampled_type) != SpvOpTypeVoid) {
     const uint32_t texel_component_type =
         _.GetComponentType(actual_result_type);
@@ -1156,16 +1174,11 @@ spv_result_t ValidateImageLod(ValidationState_t& _, const Instruction* inst) {
            << " components, but given only " << actual_coord_size;
   }
 
-  if (inst->words().size() <= 5) {
-    assert(IsImplicitLod(opcode));
-    return SPV_SUCCESS;
-  }
-
-  const uint32_t mask = inst->word(5);
+  const uint32_t mask = inst->words().size() <= 5 ? 0 : inst->word(5);
 
-  if (spvIsOpenCLEnv(_.context()->target_env)) {
-    if (opcode == SpvOpImageSampleExplicitLod) {
-      if (mask & SpvImageOperandsConstOffsetMask) {
+  if (mask & SpvImageOperandsConstOffsetMask) {
+    if (spvIsOpenCLEnv(_.context()->target_env)) {
+      if (opcode == SpvOpImageSampleExplicitLod) {
         return _.diag(SPV_ERROR_INVALID_DATA, inst)
                << "ConstOffset image operand not allowed "
                << "in the OpenCL environment.";
@@ -1174,7 +1187,7 @@ spv_result_t ValidateImageLod(ValidationState_t& _, const Instruction* inst) {
   }
 
   if (spv_result_t result =
-          ValidateImageOperands(_, inst, info, mask, /* word_index = */ 6))
+          ValidateImageOperands(_, inst, info, /* word_index = */ 6))
     return result;
 
   return SPV_SUCCESS;
@@ -1209,6 +1222,14 @@ spv_result_t ValidateImageDrefLod(ValidationState_t& _,
 
   if (spv_result_t result = ValidateImageCommon(_, inst, info)) return result;
 
+  if (info.multisampled) {
+    // When using image operands, the Sample image operand is required if and
+    // only if the image is multisampled (MS=1). The Sample image operand is
+    // only allowed for fetch, read, and write.
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "Dref sampling operation is invalid for multisample image";
+  }
+
   if (actual_result_type != info.sampled_type) {
     return _.diag(SPV_ERROR_INVALID_DATA, inst)
            << "Expected Image 'Sampled Type' to be the same as "
@@ -1235,14 +1256,8 @@ spv_result_t ValidateImageDrefLod(ValidationState_t& _,
            << "Expected Dref to be of 32-bit float type";
   }
 
-  if (inst->words().size() <= 6) {
-    assert(IsImplicitLod(opcode));
-    return SPV_SUCCESS;
-  }
-
-  const uint32_t mask = inst->word(6);
   if (spv_result_t result =
-          ValidateImageOperands(_, inst, info, mask, /* word_index = */ 7))
+          ValidateImageOperands(_, inst, info, /* word_index = */ 7))
     return result;
 
   return SPV_SUCCESS;
@@ -1313,11 +1328,8 @@ spv_result_t ValidateImageFetch(ValidationState_t& _, const Instruction* inst) {
            << " components, but given only " << actual_coord_size;
   }
 
-  if (inst->words().size() <= 5) return SPV_SUCCESS;
-
-  const uint32_t mask = inst->word(5);
   if (spv_result_t result =
-          ValidateImageOperands(_, inst, info, mask, /* word_index = */ 6))
+          ValidateImageOperands(_, inst, info, /* word_index = */ 6))
     return result;
 
   return SPV_SUCCESS;
@@ -1355,6 +1367,14 @@ spv_result_t ValidateImageGather(ValidationState_t& _,
            << "Corrupt image type definition";
   }
 
+  if (info.multisampled) {
+    // When using image operands, the Sample image operand is required if and
+    // only if the image is multisampled (MS=1). The Sample image operand is
+    // only allowed for fetch, read, and write.
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "Gather operation is invalid for multisample image";
+  }
+
   if (opcode == SpvOpImageDrefGather || opcode == SpvOpImageSparseDrefGather ||
       _.GetIdOpcode(info.sampled_type) != SpvOpTypeVoid) {
     const uint32_t result_component_type =
@@ -1403,11 +1423,8 @@ spv_result_t ValidateImageGather(ValidationState_t& _,
     }
   }
 
-  if (inst->words().size() <= 6) return SPV_SUCCESS;
-
-  const uint32_t mask = inst->word(6);
   if (spv_result_t result =
-          ValidateImageOperands(_, inst, info, mask, /* word_index = */ 7))
+          ValidateImageOperands(_, inst, info, /* word_index = */ 7))
     return result;
 
   return SPV_SUCCESS;
@@ -1496,12 +1513,10 @@ spv_result_t ValidateImageRead(ValidationState_t& _, const Instruction* inst) {
     }
   }
 
-  if (inst->words().size() <= 5) return SPV_SUCCESS;
+  const uint32_t mask = inst->words().size() <= 5 ? 0 : inst->word(5);
 
-  const uint32_t mask = inst->word(5);
-
-  if (spvIsOpenCLEnv(_.context()->target_env)) {
-    if (mask & SpvImageOperandsConstOffsetMask) {
+  if (mask & SpvImageOperandsConstOffsetMask) {
+    if (spvIsOpenCLEnv(_.context()->target_env)) {
       return _.diag(SPV_ERROR_INVALID_DATA, inst)
              << "ConstOffset image operand not allowed "
              << "in the OpenCL environment.";
@@ -1509,7 +1524,7 @@ spv_result_t ValidateImageRead(ValidationState_t& _, const Instruction* inst) {
   }
 
   if (spv_result_t result =
-          ValidateImageOperands(_, inst, info, mask, /* word_index = */ 6))
+          ValidateImageOperands(_, inst, info, /* word_index = */ 6))
     return result;
 
   return SPV_SUCCESS;
@@ -1585,9 +1600,7 @@ spv_result_t ValidateImageWrite(ValidationState_t& _, const Instruction* inst) {
     }
   }
 
-  if (inst->words().size() <= 4) {
-    return SPV_SUCCESS;
-  } else {
+  if (inst->words().size() > 4) {
     if (spvIsOpenCLEnv(_.context()->target_env)) {
       return _.diag(SPV_ERROR_INVALID_DATA, inst)
              << "Optional Image Operands are not allowed in the OpenCL "
@@ -1595,9 +1608,8 @@ spv_result_t ValidateImageWrite(ValidationState_t& _, const Instruction* inst) {
     }
   }
 
-  const uint32_t mask = inst->word(4);
   if (spv_result_t result =
-          ValidateImageOperands(_, inst, info, mask, /* word_index = */ 5))
+          ValidateImageOperands(_, inst, info, /* word_index = */ 5))
     return result;
 
   return SPV_SUCCESS;
diff --git a/test/val/val_image_test.cpp b/test/val/val_image_test.cpp
index 31f93752..e9159a1e 100644
--- a/test/val/val_image_test.cpp
+++ b/test/val/val_image_test.cpp
@@ -66,7 +66,7 @@ OpCapability ImageBuffer
 %uniform_image_f32_1d_0001
 %uniform_image_f32_1d_0002_rgba32f
 %uniform_image_f32_2d_0001
-%uniform_image_f32_2d_0011
+%uniform_image_f32_2d_0011 ; multisampled sampled
 %uniform_image_u32_2d_0001
 %uniform_image_u32_2d_0002
 %uniform_image_s32_3d_0001
@@ -1080,6 +1080,20 @@ TEST_F(ValidateImage, SampleImplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleImplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleExplicitLod %f32vec4 %simg %f32vec2_hh Sample %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Sampling operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, SampleImplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -1228,6 +1242,20 @@ TEST_F(ValidateImage, SampleExplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleExplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec2_hh Lod|Sample %f32_0 %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Sampling operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, SampleExplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -1360,19 +1388,6 @@ TEST_F(ValidateImage, LodWrongDim) {
                         "2D, 3D or Cube"));
 }
 
-TEST_F(ValidateImage, LodMultisampled) {
-  const std::string body = R"(
-%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
-%sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
-%res1 = OpImageSampleExplicitLod %f32vec4 %simg %f32vec2_00 Lod %f32_0)";
-
-  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Image Operand Lod requires 'MS' parameter to be 0"));
-}
-
 TEST_F(ValidateImage, MinLodIncompatible) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -1405,20 +1420,6 @@ TEST_F(ValidateImage, ImplicitLodWithGrad) {
           "Image Operand Grad can only be used with ExplicitLod opcodes"));
 }
 
-TEST_F(ValidateImage, SampleImplicitLod3DArrayedMultisampledSuccess) {
-  const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
-%sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res1 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000
-%res2 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 ConstOffset %s32vec3_012
-%res3 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 Offset %s32vec3_012
-)";
-
-  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
-}
-
 TEST_F(ValidateImage, SampleImplicitLodCubeArrayedSuccess) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_cube_0101 %uniform_image_f32_cube_0101
@@ -1463,20 +1464,6 @@ TEST_F(ValidateImage, SampleImplicitLodBiasWrongDim) {
                         "2D, 3D or Cube"));
 }
 
-TEST_F(ValidateImage, SampleImplicitLodBiasMultisampled) {
-  const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
-%sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res1 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 Bias %f32_0_25
-)";
-
-  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Image Operand Bias requires 'MS' parameter to be 0"));
-}
-
 TEST_F(ValidateImage, SampleExplicitLodGradDxWrongType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_cube_0101 %uniform_image_f32_cube_0101
@@ -1539,20 +1526,6 @@ TEST_F(ValidateImage, SampleExplicitLodGradDyWrongSize) {
           "Expected Image Operand Grad dy to have 3 components, but given 2"));
 }
 
-TEST_F(ValidateImage, SampleExplicitLodGradMultisampled) {
-  const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
-%sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res1 = OpImageSampleExplicitLod %f32vec4 %simg %f32vec4_0000 Grad %f32vec3_000 %f32vec3_000
-)";
-
-  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Image Operand Grad requires 'MS' parameter to be 0"));
-}
-
 TEST_F(ValidateImage, SampleImplicitLodConstOffsetCubeDim) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_cube_0101 %uniform_image_f32_cube_0101
@@ -1571,10 +1544,10 @@ TEST_F(ValidateImage, SampleImplicitLodConstOffsetCubeDim) {
 
 TEST_F(ValidateImage, SampleImplicitLodConstOffsetWrongType) {
   const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
+%img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
 %sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 ConstOffset %f32vec3_000
+%simg = OpSampledImage %type_sampled_image_f32_2d_0001 %img %sampler
+%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec2_00 ConstOffset %f32vec2_00
 )";
 
   CompileSuccessfully(GenerateShaderCode(body).c_str());
@@ -1587,26 +1560,26 @@ TEST_F(ValidateImage, SampleImplicitLodConstOffsetWrongType) {
 
 TEST_F(ValidateImage, SampleImplicitLodConstOffsetWrongSize) {
   const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
+%img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
 %sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 ConstOffset %s32vec2_01
+%simg = OpSampledImage %type_sampled_image_f32_2d_0001 %img %sampler
+%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec2_00 ConstOffset %s32vec3_012
 )";
 
   CompileSuccessfully(GenerateShaderCode(body).c_str());
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Expected Image Operand ConstOffset to have 3 "
-                        "components, but given 2"));
+              HasSubstr("Expected Image Operand ConstOffset to have 2 "
+                        "components, but given 3"));
 }
 
 TEST_F(ValidateImage, SampleImplicitLodConstOffsetNotConst) {
   const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
+%img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
 %sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0001 %img %sampler
 %offset = OpSNegate %s32vec3 %s32vec3_012
-%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 ConstOffset %offset
+%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec2_00 ConstOffset %offset
 )";
 
   CompileSuccessfully(GenerateShaderCode(body).c_str());
@@ -1633,10 +1606,10 @@ TEST_F(ValidateImage, SampleImplicitLodOffsetCubeDim) {
 
 TEST_F(ValidateImage, SampleImplicitLodOffsetWrongType) {
   const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
+%img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
 %sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 Offset %f32vec3_000
+%simg = OpSampledImage %type_sampled_image_f32_2d_0001 %img %sampler
+%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 Offset %f32vec2_00
 )";
 
   CompileSuccessfully(GenerateShaderCode(body).c_str());
@@ -1648,10 +1621,10 @@ TEST_F(ValidateImage, SampleImplicitLodOffsetWrongType) {
 
 TEST_F(ValidateImage, SampleImplicitLodOffsetWrongSize) {
   const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
+%img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
 %sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 Offset %s32vec2_01
+%simg = OpSampledImage %type_sampled_image_f32_2d_0001 %img %sampler
+%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 Offset %s32vec3_012
 )";
 
   CompileSuccessfully(GenerateShaderCode(body).c_str());
@@ -1659,15 +1632,15 @@ TEST_F(ValidateImage, SampleImplicitLodOffsetWrongSize) {
   EXPECT_THAT(
       getDiagnosticString(),
       HasSubstr(
-          "Expected Image Operand Offset to have 3 components, but given 2"));
+          "Expected Image Operand Offset to have 2 components, but given 3"));
 }
 
 TEST_F(ValidateImage, SampleImplicitLodMoreThanOneOffset) {
   const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
+%img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
 %sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 ConstOffset|Offset %s32vec3_012 %s32vec3_012
+%simg = OpSampledImage %type_sampled_image_f32_2d_0001 %img %sampler
+%res4 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 ConstOffset|Offset %s32vec2_01 %s32vec2_01
 )";
 
   CompileSuccessfully(GenerateShaderCode(body).c_str());
@@ -1706,21 +1679,6 @@ TEST_F(ValidateImage, SampleImplicitLodMinLodWrongDim) {
                         "1D, 2D, 3D or Cube"));
 }
 
-TEST_F(ValidateImage, SampleImplicitLodMinLodMultisampled) {
-  const std::string body = R"(
-%img = OpLoad %type_image_f32_3d_0111 %uniform_image_f32_3d_0111
-%sampler = OpLoad %type_sampler %uniform_sampler
-%simg = OpSampledImage %type_sampled_image_f32_3d_0111 %img %sampler
-%res1 = OpImageSampleImplicitLod %f32vec4 %simg %f32vec4_0000 MinLod %f32_0_25
-)";
-
-  CompileSuccessfully(GenerateShaderCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(
-      getDiagnosticString(),
-      HasSubstr("Image Operand MinLod requires 'MS' parameter to be 0"));
-}
-
 TEST_F(ValidateImage, SampleProjExplicitLodSuccess2D) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -1798,6 +1756,20 @@ TEST_F(ValidateImage, SampleProjExplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleProjExplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleProjExplicitLod %f32vec4 %simg %f32vec2_hh Lod|Sample %f32_1 %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Image 'MS' parameter to be 0"));
+}
+
 TEST_F(ValidateImage, SampleProjExplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -1919,6 +1891,20 @@ TEST_F(ValidateImage, SampleProjImplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleProjImplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleProjImplicitLod %f32vec4 %simg %f32vec2_hh Sample %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Expected Image 'MS' parameter to be 0"));
+}
+
 TEST_F(ValidateImage, SampleProjImplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -2026,6 +2012,21 @@ TEST_F(ValidateImage, SampleDrefImplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleDrefImplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleDrefImplicitLod %f32 %simg %f32vec2_hh %f32_1 Sample %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Dref sampling operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, SampleDrefImplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_u32_2d_0001 %uniform_image_u32_2d_0001
@@ -2149,6 +2150,21 @@ TEST_F(ValidateImage, SampleDrefExplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleDrefExplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleDrefExplicitLod %f32 %simg %f32vec2_hh %f32_1 Lod|Sample %f32_1 %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Dref sampling operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, SampleDrefExplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_s32_3d_0001 %uniform_image_s32_3d_0001
@@ -2273,6 +2289,21 @@ TEST_F(ValidateImage, SampleProjDrefImplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleProjDrefImplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleDrefExplicitLod %f32 %simg %f32vec2_hh %f32_1 Sample %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Dref sampling operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, SampleProjDrefImplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -2396,6 +2427,21 @@ TEST_F(ValidateImage, SampleProjDrefExplicitLodNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, SampleProjDrefExplicitLodMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageSampleDrefExplicitLod %f32 %simg %f32vec2_hh %f32_1 Lod|Sample %f32_1 %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(
+      getDiagnosticString(),
+      HasSubstr("Dref sampling operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, SampleProjDrefExplicitLodWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_1d_0001 %uniform_image_f32_1d_0001
@@ -2472,6 +2518,23 @@ OpExtension "SPV_KHR_vulkan_memory_model"
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
 }
 
+TEST_F(ValidateImage, FetchMultisampledSuccess) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%res1 = OpImageFetch %f32vec4 %img %u32vec2_01 Sample %u32_1
+%res2 = OpImageFetch %f32vec4 %img %u32vec2_01 Sample|NonPrivateTexelKHR %u32_1
+)";
+
+  const std::string extra = R"(
+OpCapability VulkanMemoryModelKHR
+OpExtension "SPV_KHR_vulkan_memory_model"
+)";
+  CompileSuccessfully(GenerateShaderCode(body, extra, "Fragment", "",
+                                         SPV_ENV_UNIVERSAL_1_3, "VulkanKHR")
+                          .c_str());
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
+}
+
 TEST_F(ValidateImage, FetchWrongResultType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_rect_0001 %uniform_image_f32_rect_0001
@@ -2611,6 +2674,21 @@ TEST_F(ValidateImage, FetchLodNotInt) {
                         "with OpImageFetch"));
 }
 
+TEST_F(ValidateImage, FetchMultisampledMissingSample) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%res1 = OpImageFetch %f32vec4 %img %u32vec2_01
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions())
+      << GenerateShaderCode(body);
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Image Operand Sample is required for operation on "
+                        "multi-sampled image"))
+      << getDiagnosticString();
+}
+
 TEST_F(ValidateImage, GatherSuccess) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001
@@ -2672,6 +2750,20 @@ TEST_F(ValidateImage, GatherNotSampledImage) {
       HasSubstr("Expected Sampled Image to be of type OpTypeSampledImage"));
 }
 
+TEST_F(ValidateImage, GatherMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageGather %f32vec4 %simg %f32vec4_0000 %u32_1 Sample %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Gather operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, GatherWrongSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_cube_0101 %uniform_image_f32_cube_0101
@@ -2887,6 +2979,20 @@ OpExtension "SPV_KHR_vulkan_memory_model"
   ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_UNIVERSAL_1_3));
 }
 
+TEST_F(ValidateImage, DrefGatherMultisampleError) {
+  const std::string body = R"(
+%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
+%sampler = OpLoad %type_sampler %uniform_sampler
+%simg = OpSampledImage %type_sampled_image_f32_2d_0011 %img %sampler
+%res1 = OpImageDrefGather %f32vec4 %simg %f32vec4_0000 %f32_1 Sample %u32_1
+)";
+
+  CompileSuccessfully(GenerateShaderCode(body).c_str());
+  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("Gather operation is invalid for multisample image"));
+}
+
 TEST_F(ValidateImage, DrefGatherVoidSampledType) {
   const std::string body = R"(
 %img = OpLoad %type_image_void_2d_0001 %uniform_image_void_2d_0001
@@ -3360,7 +3466,7 @@ OpImageWrite %img %u32vec2_01 %f32vec4_0000 Sample %f32_1
               HasSubstr("Expected Image Operand Sample to be int scalar"));
 }
 
-TEST_F(ValidateImage, SampleNotMultisampled) {
+TEST_F(ValidateImage, WriteSampleNotMultisampled) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0002 %uniform_image_f32_2d_0002
 OpImageWrite %img %u32vec2_01 %f32vec4_0000 Sample %u32_1
@@ -3385,9 +3491,7 @@ TEST_F(ValidateImage, SampleWrongOpcode) {
   CompileSuccessfully(GenerateShaderCode(body).c_str());
   ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
   EXPECT_THAT(getDiagnosticString(),
-              HasSubstr("Image Operand Sample can only be used with "
-                        "OpImageFetch, OpImageRead, OpImageWrite, "
-                        "OpImageSparseFetch and OpImageSparseRead"));
+              HasSubstr("Sampling operation is invalid for multisample image"));
 }
 
 TEST_F(ValidateImage, SampleImageToImageSuccess) {
@@ -3592,17 +3696,6 @@ TEST_F(ValidateImage, QuerySizeLodWrongImageDim) {
               HasSubstr("Image 'Dim' must be 1D, 2D, 3D or Cube"));
 }
 
-TEST_F(ValidateImage, QuerySizeLodMultisampled) {
-  const std::string body = R"(
-%img = OpLoad %type_image_f32_2d_0011 %uniform_image_f32_2d_0011
-%res1 = OpImageQuerySizeLod %u32vec2 %img %u32_1
-)";
-
-  CompileSuccessfully(GenerateKernelCode(body).c_str());
-  ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions());
-  EXPECT_THAT(getDiagnosticString(), HasSubstr("Image 'MS' must be 0"));
-}
-
 TEST_F(ValidateImage, QuerySizeLodWrongLodType) {
   const std::string body = R"(
 %img = OpLoad %type_image_f32_2d_0001 %uniform_image_f32_2d_0001