From c040bd3ae5fac4eeb89fbd7a49a5f4364929dc6a Mon Sep 17 00:00:00 2001 From: sfricke-samsung <46493288+sfricke-samsung@users.noreply.github.com> Date: Wed, 17 Mar 2021 07:00:11 -0700 Subject: [PATCH] spirv-val: Add Vulkan Execution Scope checks (#4183) * spirv-val: Add Vulkan Execution Scope checks --- source/val/validate_scopes.cpp | 50 +++++++++++++++++---- source/val/validation_state.cpp | 6 +++ test/val/val_barriers_test.cpp | 76 +++++++++++++++++++++++++------- test/val/val_decoration_test.cpp | 2 + 4 files changed, 110 insertions(+), 24 deletions(-) diff --git a/source/val/validate_scopes.cpp b/source/val/validate_scopes.cpp index a92f7fd31..29ba58317 100644 --- a/source/val/validate_scopes.cpp +++ b/source/val/validate_scopes.cpp @@ -105,21 +105,30 @@ spv_result_t ValidateExecutionScope(ValidationState_t& _, } } - // If OpControlBarrier is used in fragment, vertex, tessellation evaluation, - // or geometry stages, the execution Scope must be Subgroup. + // OpControlBarrier must only use Subgroup execution scope for a subset of + // execution models. if (opcode == SpvOpControlBarrier && value != SpvScopeSubgroup) { + std::string errorVUID = _.VkErrorID(4682); _.function(inst->function()->id()) - ->RegisterExecutionModelLimitation([](SpvExecutionModel model, - std::string* message) { + ->RegisterExecutionModelLimitation([errorVUID]( + SpvExecutionModel model, + std::string* message) { if (model == SpvExecutionModelFragment || model == SpvExecutionModelVertex || model == SpvExecutionModelGeometry || - model == SpvExecutionModelTessellationEvaluation) { + model == SpvExecutionModelTessellationEvaluation || + model == SpvExecutionModelRayGenerationKHR || + model == SpvExecutionModelIntersectionKHR || + model == SpvExecutionModelAnyHitKHR || + model == SpvExecutionModelClosestHitKHR || + model == SpvExecutionModelMissKHR) { if (message) { *message = - "in Vulkan evironment, OpControlBarrier execution scope " - "must be Subgroup for Fragment, Vertex, Geometry and " - "TessellationEvaluation execution models"; + errorVUID + + "in Vulkan environment, OpControlBarrier execution scope " + "must be Subgroup for Fragment, Vertex, Geometry, " + "TessellationEvaluation, RayGeneration, Intersection, " + "AnyHit, ClosestHit, and Miss execution models"; } return false; } @@ -127,11 +136,34 @@ spv_result_t ValidateExecutionScope(ValidationState_t& _, }); } + // Only subset of execution models support Workgroup. + if (value == SpvScopeWorkgroup) { + std::string errorVUID = _.VkErrorID(4637); + _.function(inst->function()->id()) + ->RegisterExecutionModelLimitation( + [errorVUID](SpvExecutionModel model, std::string* message) { + if (model != SpvExecutionModelTaskNV && + model != SpvExecutionModelMeshNV && + model != SpvExecutionModelTessellationControl && + model != SpvExecutionModelGLCompute) { + if (message) { + *message = + errorVUID + + "in Vulkan environment, Workgroup execution scope is " + "only for TaskNV, MeshNV, TessellationControl, and " + "GLCompute execution models"; + } + return false; + } + return true; + }); + } + // Vulkan generic rules // Scope for execution must be limited to Workgroup or Subgroup if (value != SpvScopeWorkgroup && value != SpvScopeSubgroup) { return _.diag(SPV_ERROR_INVALID_DATA, inst) - << spvOpcodeString(opcode) + << _.VkErrorID(4636) << spvOpcodeString(opcode) << ": in Vulkan environment Execution Scope is limited to " << "Workgroup and Subgroup"; } diff --git a/source/val/validation_state.cpp b/source/val/validation_state.cpp index 2386a2b3b..b56d5e0af 100644 --- a/source/val/validation_state.cpp +++ b/source/val/validation_state.cpp @@ -1680,6 +1680,10 @@ std::string ValidationState_t::VkErrorID(uint32_t id, return VUID_WRAP(VUID-StandaloneSpirv-None-04634); case 4635: return VUID_WRAP(VUID-StandaloneSpirv-None-04635); + case 4636: + return VUID_WRAP(VUID-StandaloneSpirv-None-04636); + case 4637: + return VUID_WRAP(VUID-StandaloneSpirv-None-04637); case 4638: return VUID_WRAP(VUID-StandaloneSpirv-None-04638); case 4639: @@ -1720,6 +1724,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id, return VUID_WRAP(VUID-StandaloneSpirv-FPRoundingMode-04675); case 4677: return VUID_WRAP(VUID-StandaloneSpirv-Invariant-04677); + case 4682: + return VUID_WRAP(VUID-StandaloneSpirv-OpControlBarrier-04682); case 4683: return VUID_WRAP(VUID-StandaloneSpirv-LocalSize-04683); case 4685: diff --git a/test/val/val_barriers_test.cpp b/test/val/val_barriers_test.cpp index c36bd1952..1178ca02d 100644 --- a/test/val/val_barriers_test.cpp +++ b/test/val/val_barriers_test.cpp @@ -345,6 +345,8 @@ OpControlBarrier %device %workgroup %none CompileSuccessfully(GenerateShaderCode(body), SPV_ENV_VULKAN_1_0); ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_0)); + EXPECT_THAT(getDiagnosticString(), + AnyVUID("VUID-StandaloneSpirv-None-04636")); EXPECT_THAT(getDiagnosticString(), HasSubstr("ControlBarrier: in Vulkan environment Execution Scope " "is limited to Workgroup and Subgroup")); @@ -388,9 +390,10 @@ OpControlBarrier %subgroup %cross_device %none "cannot be CrossDevice")); } -TEST_F(ValidateBarriers, OpControlBarrierVulkan1p1WorkgroupNonComputeFailure) { +TEST_F(ValidateBarriers, + OpControlBarrierVulkan1p1WorkgroupNonComputeMemoryFailure) { const std::string body = R"( -OpControlBarrier %workgroup %workgroup %acquire +OpControlBarrier %subgroup %workgroup %acquire )"; CompileSuccessfully(GenerateVulkanVertexShaderCode(body), SPV_ENV_VULKAN_1_1); @@ -402,7 +405,23 @@ OpControlBarrier %workgroup %workgroup %acquire "and GLCompute execution model")); } -TEST_F(ValidateBarriers, OpControlBarrierVulkan1p1WorkgroupNonComputeSuccess) { +TEST_F(ValidateBarriers, + OpControlBarrierVulkan1p1WorkgroupNonComputeExecutionFailure) { + const std::string body = R"( +OpControlBarrier %workgroup %subgroup %acquire +)"; + + CompileSuccessfully(GenerateVulkanVertexShaderCode(body), SPV_ENV_VULKAN_1_1); + EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1)); + EXPECT_THAT(getDiagnosticString(), + AnyVUID("VUID-StandaloneSpirv-None-04637")); + EXPECT_THAT(getDiagnosticString(), + HasSubstr("in Vulkan environment, Workgroup execution scope is " + "only for TaskNV, MeshNV, TessellationControl, and " + "GLCompute execution models")); +} + +TEST_F(ValidateBarriers, OpControlBarrierVulkan1p1WorkgroupComputeSuccess) { const std::string body = R"( OpControlBarrier %workgroup %workgroup %acquire )"; @@ -411,6 +430,15 @@ OpControlBarrier %workgroup %workgroup %acquire ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_1)); } +TEST_F(ValidateBarriers, OpControlBarrierVulkan1p1WorkgroupNonComputeSuccess) { + const std::string body = R"( +OpControlBarrier %subgroup %subgroup %acquire +)"; + + CompileSuccessfully(GenerateVulkanVertexShaderCode(body), SPV_ENV_VULKAN_1_1); + ASSERT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_1)); +} + TEST_F(ValidateBarriers, OpControlBarrierVulkanInvocationSuccess) { const std::string body = R"( OpControlBarrier %workgroup %invocation %none @@ -483,9 +511,13 @@ OpControlBarrier %workgroup %workgroup %acquire_release SPV_ENV_VULKAN_1_1); ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1)); EXPECT_THAT(getDiagnosticString(), - HasSubstr("OpControlBarrier execution scope must be Subgroup for " - "Fragment, Vertex, Geometry and TessellationEvaluation " - "execution models")); + AnyVUID("VUID-StandaloneSpirv-OpControlBarrier-04682")); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr( + "OpControlBarrier execution scope must be Subgroup for Fragment, " + "Vertex, Geometry, TessellationEvaluation, RayGeneration, " + "Intersection, AnyHit, ClosestHit, and Miss execution models")); } TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionFragment1p0) { @@ -521,9 +553,13 @@ OpControlBarrier %workgroup %workgroup %acquire_release SPV_ENV_VULKAN_1_1); ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1)); EXPECT_THAT(getDiagnosticString(), - HasSubstr("OpControlBarrier execution scope must be Subgroup for " - "Fragment, Vertex, Geometry and TessellationEvaluation " - "execution models")); + AnyVUID("VUID-StandaloneSpirv-OpControlBarrier-04682")); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr( + "OpControlBarrier execution scope must be Subgroup for Fragment, " + "Vertex, Geometry, TessellationEvaluation, RayGeneration, " + "Intersection, AnyHit, ClosestHit, and Miss execution models")); } TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionVertex1p0) { @@ -561,9 +597,13 @@ OpControlBarrier %workgroup %workgroup %acquire_release SPV_ENV_VULKAN_1_1); ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1)); EXPECT_THAT(getDiagnosticString(), - HasSubstr("OpControlBarrier execution scope must be Subgroup for " - "Fragment, Vertex, Geometry and TessellationEvaluation " - "execution models")); + AnyVUID("VUID-StandaloneSpirv-OpControlBarrier-04682")); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr( + "OpControlBarrier execution scope must be Subgroup for Fragment, " + "Vertex, Geometry, TessellationEvaluation, RayGeneration, " + "Intersection, AnyHit, ClosestHit, and Miss execution models")); } TEST_F(ValidateBarriers, OpControlBarrierSubgroupExecutionGeometry1p0) { @@ -604,9 +644,13 @@ OpControlBarrier %workgroup %workgroup %acquire_release SPV_ENV_VULKAN_1_1); ASSERT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_1)); EXPECT_THAT(getDiagnosticString(), - HasSubstr("OpControlBarrier execution scope must be Subgroup for " - "Fragment, Vertex, Geometry and TessellationEvaluation " - "execution models")); + AnyVUID("VUID-StandaloneSpirv-OpControlBarrier-04682")); + EXPECT_THAT( + getDiagnosticString(), + HasSubstr( + "OpControlBarrier execution scope must be Subgroup for Fragment, " + "Vertex, Geometry, TessellationEvaluation, RayGeneration, " + "Intersection, AnyHit, ClosestHit, and Miss execution models")); } TEST_F(ValidateBarriers, @@ -1487,6 +1531,8 @@ TEST_F(ValidateBarriers, OpControlBarrierShaderCallRayGenFailure) { SPV_ENV_VULKAN_1_1); ASSERT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_1)); + EXPECT_THAT(getDiagnosticString(), + AnyVUID("VUID-StandaloneSpirv-None-04636")); EXPECT_THAT(getDiagnosticString(), HasSubstr("in Vulkan environment Execution Scope is limited to " "Workgroup and Subgroup")); diff --git a/test/val/val_decoration_test.cpp b/test/val/val_decoration_test.cpp index cbf6d7c1c..096fd172d 100644 --- a/test/val/val_decoration_test.cpp +++ b/test/val/val_decoration_test.cpp @@ -4991,6 +4991,8 @@ TEST_F(ValidateDecorations, UniformDecorationWithScopeIdV14VulkanEnv) { CompileSuccessfully(spirv, SPV_ENV_VULKAN_1_1_SPIRV_1_4); EXPECT_EQ(SPV_ERROR_INVALID_DATA, ValidateInstructions(SPV_ENV_VULKAN_1_1_SPIRV_1_4)); + EXPECT_THAT(getDiagnosticString(), + AnyVUID("VUID-StandaloneSpirv-None-04636")); EXPECT_THAT(getDiagnosticString(), HasSubstr(": in Vulkan environment Execution Scope is limited to " "Workgroup and Subgroup"));