From c8b09744c6a15f3586c26351376bf5c6f656e89f Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Wed, 1 May 2019 13:29:39 -0400 Subject: [PATCH] Add validation specific to OpExecutionModeId (#2536) Fixes #1565 --- source/val/validate_mode_setting.cpp | 30 +++++ test/val/val_modes_test.cpp | 165 +++++++++++++++++++++++++++ 2 files changed, 195 insertions(+) diff --git a/source/val/validate_mode_setting.cpp b/source/val/validate_mode_setting.cpp index 72a8869a0..943629d11 100644 --- a/source/val/validate_mode_setting.cpp +++ b/source/val/validate_mode_setting.cpp @@ -235,6 +235,36 @@ spv_result_t ValidateExecutionMode(ValidationState_t& _, } const auto mode = inst->GetOperandAs(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(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: diff --git a/test/val/val_modes_test.cpp b/test/val/val_modes_test.cpp index 12f2b8ce9..bc157a4f1 100644 --- a/test/val/val_modes_test.cpp +++ b/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