Add validation specific to OpExecutionModeId (#2536)

Fixes #1565

source/val/validate_mode_setting.cpp

@@ -235,6 +235,36 @@ spv_result_t ValidateExecutionMode(ValidationState_t& _,
   }
 
   const auto mode = inst->GetOperandAs<SpvExecutionMode>(1);
+  if (inst->opcode() == SpvOpExecutionModeId) {
+    size_t operand_count = inst->operands().size();
+    for (size_t i = 2; i < operand_count; ++i) {
+      const auto operand_id = inst->GetOperandAs<uint32_t>(2);
+      const auto* operand_inst = _.FindDef(operand_id);
+      if (mode == SpvExecutionModeSubgroupsPerWorkgroupId ||
+          mode == SpvExecutionModeLocalSizeHintId ||
+          mode == SpvExecutionModeLocalSizeId) {
+        if (!spvOpcodeIsConstant(operand_inst->opcode())) {
+          return _.diag(SPV_ERROR_INVALID_ID, inst)
+                 << "For OpExecutionModeId all Extra Operand ids must be "
+                    "constant "
+                    "instructions.";
+        }
+      } else {
+        return _.diag(SPV_ERROR_INVALID_ID, inst)
+               << "OpExecutionModeId is only valid when the Mode operand is an "
+                  "execution mode that takes Extra Operands that are id "
+                  "operands.";
+      }
+    }
+  } else if (mode == SpvExecutionModeSubgroupsPerWorkgroupId ||
+             mode == SpvExecutionModeLocalSizeHintId ||
+             mode == SpvExecutionModeLocalSizeId) {
+    return _.diag(SPV_ERROR_INVALID_DATA, inst)
+           << "OpExecutionMode is only valid when the Mode operand is an "
+              "execution mode that takes no Extra Operands, or takes Extra "
+              "Operands that are not id operands.";
+  }
+
   const auto* models = _.GetExecutionModels(entry_point_id);
   switch (mode) {
     case SpvExecutionModeInvocations:

test/val/val_modes_test.cpp

@@ -837,6 +837,171 @@ OpExecutionModeId %main LocalSizeId %int_1 %int_1 %int_1
   EXPECT_THAT(SPV_SUCCESS, ValidateInstructions(env));
 }
 
+TEST_F(ValidateModeExecution, ExecModeSubgroupsPerWorkgroupIdBad) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability SubgroupDispatch
+OpMemoryModel Logical GLSL450
+OpEntryPoint Vertex %main "main"
+OpExecutionMode %main SubgroupsPerWorkgroupId %int_1
+%int = OpTypeInt 32 0
+%int_1 = OpConstant %int 1
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpExecutionMode is only valid when the Mode operand "
+                        "is an execution mode that takes no Extra Operands"));
+}
+
+TEST_F(ValidateModeExecution, ExecModeIdSubgroupsPerWorkgroupIdGood) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability SubgroupDispatch
+OpMemoryModel Logical GLSL450
+OpEntryPoint Vertex %main "main"
+OpExecutionModeId %main SubgroupsPerWorkgroupId %int_1
+%int = OpTypeInt 32 0
+%int_1 = OpConstant %int 1
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_SUCCESS, ValidateInstructions(env));
+}
+
+TEST_F(ValidateModeExecution, ExecModeIdSubgroupsPerWorkgroupIdNonConstantBad) {
+  const std::string spirv = R"(
+OpCapability Shader
+OpCapability SubgroupDispatch
+OpMemoryModel Logical GLSL450
+OpEntryPoint Vertex %main "main"
+OpExecutionModeId %main SubgroupsPerWorkgroupId %int_1
+%int = OpTypeInt 32 0
+%int_ptr = OpTypePointer Private %int
+%int_1 = OpVariable %int_ptr Private
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_ERROR_INVALID_ID, ValidateInstructions(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("For OpExecutionModeId all Extra Operand ids must be "
+                        "constant instructions."));
+}
+
+TEST_F(ValidateModeExecution, ExecModeLocalSizeHintIdBad) {
+  const std::string spirv = R"(
+OpCapability Kernel
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Kernel %main "main"
+OpExecutionMode %main LocalSizeHintId %int_1
+%int = OpTypeInt 32 0
+%int_1 = OpConstant %int 1
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpExecutionMode is only valid when the Mode operand "
+                        "is an execution mode that takes no Extra Operands"));
+}
+
+TEST_F(ValidateModeExecution, ExecModeIdLocalSizeHintIdGood) {
+  const std::string spirv = R"(
+OpCapability Kernel
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Kernel %main "main"
+OpExecutionModeId %main LocalSizeHintId %int_1
+%int = OpTypeInt 32 0
+%int_1 = OpConstant %int 1
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_SUCCESS, ValidateInstructions(env));
+}
+
+TEST_F(ValidateModeExecution, ExecModeIdLocalSizeHintIdNonConstantBad) {
+  const std::string spirv = R"(
+OpCapability Kernel
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Vertex %main "main"
+OpExecutionModeId %main LocalSizeHintId %int_1
+%int = OpTypeInt 32 0
+%int_ptr = OpTypePointer Private %int
+%int_1 = OpVariable %int_ptr Private
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_ERROR_INVALID_ID, ValidateInstructions(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("For OpExecutionModeId all Extra Operand ids must be "
+                        "constant instructions."));
+}
+
+TEST_F(ValidateModeExecution, ExecModeLocalSizeIdBad) {
+  const std::string spirv = R"(
+OpCapability Kernel
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Kernel %main "main"
+OpExecutionMode %main LocalSizeId %int_1 %int_1 %int_1
+%int = OpTypeInt 32 0
+%int_1 = OpConstant %int 1
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_ERROR_INVALID_DATA, ValidateInstructions(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("OpExecutionMode is only valid when the Mode operand "
+                        "is an execution mode that takes no Extra Operands"));
+}
+
+TEST_F(ValidateModeExecution, ExecModeIdLocalSizeIdGood) {
+  const std::string spirv = R"(
+OpCapability Kernel
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Kernel %main "main"
+OpExecutionModeId %main LocalSizeId %int_1 %int_1 %int_1
+%int = OpTypeInt 32 0
+%int_1 = OpConstant %int 1
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_SUCCESS, ValidateInstructions(env));
+}
+
+TEST_F(ValidateModeExecution, ExecModeIdLocalSizeIdNonConstantBad) {
+  const std::string spirv = R"(
+OpCapability Kernel
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Vertex %main "main"
+OpExecutionModeId %main LocalSizeId %int_1 %int_1 %int_1
+%int = OpTypeInt 32 0
+%int_ptr = OpTypePointer Private %int
+%int_1 = OpVariable %int_ptr Private
+)" + kVoidFunction;
+
+  spv_target_env env = SPV_ENV_UNIVERSAL_1_3;
+  CompileSuccessfully(spirv, env);
+  EXPECT_THAT(SPV_ERROR_INVALID_ID, ValidateInstructions(env));
+  EXPECT_THAT(getDiagnosticString(),
+              HasSubstr("For OpExecutionModeId all Extra Operand ids must be "
+                        "constant instructions."));
+}
+
 }  // namespace
 }  // namespace val
 }  // namespace spvtools