spirv-val: Add constant check to mesh execution mode (#6560)
We discussed in https://gitlab.khronos.org/vulkan/vulkan/-/issues/4694
that having a `SetMeshOutputsEXT` with bogus constant values should be
disallowed, even if never taken as it currently causes various drivers
issues trying to lower that code
I added test to ensure that with spec constants, this is only validated
when they are frozen
diff --git a/source/val/validate_instruction.cpp b/source/val/validate_instruction.cpp
index 26273af..7b3b3af 100644
--- a/source/val/validate_instruction.cpp
+++ b/source/val/validate_instruction.cpp
@@ -491,6 +491,10 @@
spv::ExecutionMode::OutputPrimitivesEXT) {
_.RegisterEntryPointOutputPrimitivesEXT(entry_point, inst);
}
+ if (inst->GetOperandAs<spv::ExecutionMode>(1) ==
+ spv::ExecutionMode::OutputVertices) {
+ _.RegisterEntryPointOutputVertices(entry_point, inst);
+ }
} else if (opcode == spv::Op::OpVariable) {
const auto storage_class = inst->GetOperandAs<spv::StorageClass>(2);
if (auto error = LimitCheckNumVars(_, inst->id(), storage_class)) {
diff --git a/source/val/validate_mesh_shading.cpp b/source/val/validate_mesh_shading.cpp
index 121f339..dd9bb4a 100644
--- a/source/val/validate_mesh_shading.cpp
+++ b/source/val/validate_mesh_shading.cpp
@@ -14,6 +14,8 @@
// Validates ray query instructions from SPV_KHR_ray_query
+#include <string>
+
#include "source/val/instruction.h"
#include "source/val/validate.h"
#include "source/val/validation_state.h"
@@ -118,6 +120,50 @@
<< "Primitive Count must be a 32-bit unsigned int scalar";
}
+ // Will only validate if constants are used (or spec constant frozen)
+ uint64_t vertex_count_value = 0;
+ if (_.EvalConstantValUint64(inst->GetOperandAs<uint32_t>(0),
+ &vertex_count_value)) {
+ _.function(inst->function()->id())
+ ->RegisterLimitation(
+ [vertex_count_value](const ValidationState_t& state,
+ const Function* entry_point,
+ std::string* message) {
+ const uint32_t output_vertices =
+ state.GetOutputVertices(entry_point->id());
+ if (vertex_count_value > output_vertices) {
+ *message =
+ "OpSetMeshOutputsEXT Vertex Count (" +
+ std::to_string(vertex_count_value) +
+ ") is larger than the OutputVertices in OpExecutionMode (" +
+ std::to_string(output_vertices) + ").";
+ return false;
+ }
+ return true;
+ });
+ }
+ uint64_t primitive_count_value = 0;
+ if (_.EvalConstantValUint64(inst->GetOperandAs<uint32_t>(1),
+ &primitive_count_value)) {
+ _.function(inst->function()->id())
+ ->RegisterLimitation(
+ [primitive_count_value](const ValidationState_t& state,
+ const Function* entry_point,
+ std::string* message) {
+ const uint32_t output_primitives =
+ state.GetOutputPrimitivesEXT(entry_point->id());
+ if (primitive_count_value > output_primitives) {
+ *message = "OpSetMeshOutputsEXT Primitive Count (" +
+ std::to_string(primitive_count_value) +
+ ") is larger than the OutputPrimitivesEXT in "
+ "OpExecutionMode (" +
+ std::to_string(output_primitives) + ").";
+ return false;
+ }
+ return true;
+ });
+ }
+
return SPV_SUCCESS;
}
diff --git a/source/val/validation_state.h b/source/val/validation_state.h
index 5601224..b3c5359 100644
--- a/source/val/validation_state.h
+++ b/source/val/validation_state.h
@@ -271,7 +271,7 @@
}
/// Returns the maximum number of primitives mesh shader can emit
- uint32_t GetOutputPrimitivesEXT(uint32_t entry_point) {
+ uint32_t GetOutputPrimitivesEXT(uint32_t entry_point) const {
auto entry = entry_point_to_output_primitives_.find(entry_point);
if (entry != entry_point_to_output_primitives_.end()) {
auto inst = entry->second;
@@ -280,6 +280,23 @@
return 0;
}
+ /// Registers that the entry point maximum number of vertices
+ /// mesh shader will ever emit
+ void RegisterEntryPointOutputVertices(uint32_t entry_point,
+ const Instruction* inst) {
+ entry_point_to_output_vertices_[entry_point] = inst;
+ }
+
+ /// Returns the maximum number of primitives mesh shader can emit
+ uint32_t GetOutputVertices(uint32_t entry_point) const {
+ auto entry = entry_point_to_output_vertices_.find(entry_point);
+ if (entry != entry_point_to_output_vertices_.end()) {
+ auto inst = entry->second;
+ return inst->GetOperandAs<uint32_t>(2);
+ }
+ return 0;
+ }
+
/// Returns whether the entry point declares its local size
bool EntryPointHasLocalSizeOrId(uint32_t entry_point) const {
return entry_point_to_local_size_or_id_.find(entry_point) !=
@@ -1132,6 +1149,10 @@
std::unordered_map<uint32_t, const Instruction*>
entry_point_to_output_primitives_;
+ // Mapping entry point -> OutputVertices execution mode instruction
+ std::unordered_map<uint32_t, const Instruction*>
+ entry_point_to_output_vertices_;
+
/// Mapping function -> array of entry points inside this
/// module which can (indirectly) call the function.
std::unordered_map<uint32_t, std::vector<uint32_t>> function_to_entry_points_;
diff --git a/test/val/val_mesh_shading_test.cpp b/test/val/val_mesh_shading_test.cpp
index d12c27d..95eb5e8 100644
--- a/test/val/val_mesh_shading_test.cpp
+++ b/test/val/val_mesh_shading_test.cpp
@@ -1383,6 +1383,159 @@
"explicitly laid out storage class"));
}
+// https://godbolt.org/z/8s5W19xoc
+TEST_F(ValidateMeshShading, SetMeshOutputsConstantInCondition) {
+ const std::string body = R"(
+ OpCapability MeshShadingEXT
+ OpExtension "SPV_EXT_mesh_shader"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint MeshEXT %main "main" %_ %gl_MeshVerticesEXT %gl_PrimitiveTriangleIndicesEXT
+ OpExecutionModeId %main LocalSizeId %uint_1 %uint_1 %uint_1
+ OpExecutionMode %main OutputVertices 3
+ OpExecutionMode %main OutputPrimitivesEXT 1
+ OpExecutionMode %main OutputTrianglesEXT
+ OpDecorate %SSBO Block
+ OpMemberDecorate %SSBO 0 Offset 0
+ OpMemberDecorate %SSBO 1 Offset 4
+ OpMemberDecorate %SSBO 2 Offset 8
+ OpDecorate %_ Binding 0
+ OpDecorate %_ DescriptorSet 0
+ OpDecorate %gl_MeshPerVertexEXT Block
+ OpMemberDecorate %gl_MeshPerVertexEXT 0 BuiltIn Position
+ OpMemberDecorate %gl_MeshPerVertexEXT 1 BuiltIn PointSize
+ OpMemberDecorate %gl_MeshPerVertexEXT 2 BuiltIn ClipDistance
+ OpMemberDecorate %gl_MeshPerVertexEXT 3 BuiltIn CullDistance
+ OpDecorate %gl_PrimitiveTriangleIndicesEXT BuiltIn PrimitiveTriangleIndicesEXT
+ %void = OpTypeVoid
+ %4 = OpTypeFunction %void
+ %uint = OpTypeInt 32 0
+ %uint_1 = OpConstant %uint 1
+ %SSBO = OpTypeStruct %uint %uint %uint
+%_ptr_StorageBuffer_SSBO = OpTypePointer StorageBuffer %SSBO
+ %_ = OpVariable %_ptr_StorageBuffer_SSBO StorageBuffer
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+%_ptr_StorageBuffer_uint = OpTypePointer StorageBuffer %uint
+ %bool = OpTypeBool
+ %uint_500 = OpConstant %uint 500
+ %int_1 = OpConstant %int 1
+ %int_2 = OpConstant %int 2
+ %float = OpTypeFloat 32
+ %v4float = OpTypeVector %float 4
+%_arr_float_uint_1 = OpTypeArray %float %uint_1
+%gl_MeshPerVertexEXT = OpTypeStruct %v4float %float %_arr_float_uint_1 %_arr_float_uint_1
+ %uint_3 = OpConstant %uint 3
+%_arr_gl_MeshPerVertexEXT_uint_3 = OpTypeArray %gl_MeshPerVertexEXT %uint_3
+%_ptr_Output__arr_gl_MeshPerVertexEXT_uint_3 = OpTypePointer Output %_arr_gl_MeshPerVertexEXT_uint_3
+%gl_MeshVerticesEXT = OpVariable %_ptr_Output__arr_gl_MeshPerVertexEXT_uint_3 Output
+ %float_0 = OpConstant %float 0
+ %38 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+ %v3uint = OpTypeVector %uint 3
+%_arr_v3uint_uint_1 = OpTypeArray %v3uint %uint_1
+%_ptr_Output__arr_v3uint_uint_1 = OpTypePointer Output %_arr_v3uint_uint_1
+%gl_PrimitiveTriangleIndicesEXT = OpVariable %_ptr_Output__arr_v3uint_uint_1 Output
+ %uint_0 = OpConstant %uint 0
+ %uint_2 = OpConstant %uint 2
+ %47 = OpConstantComposite %v3uint %uint_0 %uint_1 %uint_2
+%_ptr_Output_v3uint = OpTypePointer Output %v3uint
+ %main = OpFunction %void None %4
+ %6 = OpLabel
+ %15 = OpAccessChain %_ptr_StorageBuffer_uint %_ %int_0
+ %16 = OpLoad %uint %15
+ %18 = OpIEqual %bool %16 %uint_1
+ OpSelectionMerge %20 None
+ OpBranchConditional %18 %19 %22
+ %19 = OpLabel
+ OpSetMeshOutputsEXT %uint_500 %uint_500
+ OpBranch %20
+ %22 = OpLabel
+ %24 = OpAccessChain %_ptr_StorageBuffer_uint %_ %int_1
+ %25 = OpLoad %uint %24
+ %27 = OpAccessChain %_ptr_StorageBuffer_uint %_ %int_2
+ %28 = OpLoad %uint %27
+ OpSetMeshOutputsEXT %25 %28
+ OpBranch %20
+ %20 = OpLabel
+ %40 = OpAccessChain %_ptr_Output_v4float %gl_MeshVerticesEXT %int_0 %int_0
+ OpStore %40 %38
+ %49 = OpAccessChain %_ptr_Output_v3uint %gl_PrimitiveTriangleIndicesEXT %int_0
+ OpStore %49 %47
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(body, SPV_ENV_VULKAN_1_3);
+ EXPECT_EQ(SPV_ERROR_INVALID_ID, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+ EXPECT_THAT(
+ getDiagnosticString(),
+ HasSubstr("OpSetMeshOutputsEXT Vertex Count (500) is larger than the "
+ "OutputVertices in OpExecutionMode (3).\nOpSetMeshOutputsEXT "
+ "Primitive Count (500) is larger than the OutputPrimitivesEXT "
+ "in OpExecutionMode (1)"));
+}
+
+// https://godbolt.org/z/TzaKxYGrY (allowed until spec constants are frozen)
+TEST_F(ValidateMeshShading, SetMeshOutputsSpecConstants) {
+ const std::string body = R"(
+ OpCapability MeshShadingEXT
+ OpExtension "SPV_EXT_mesh_shader"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint MeshEXT %main "main" %gl_MeshVerticesEXT %gl_PrimitiveTriangleIndicesEXT
+ OpExecutionModeId %main LocalSizeId %uint_1 %uint_1 %uint_1
+ OpExecutionMode %main OutputVertices 3
+ OpExecutionMode %main OutputPrimitivesEXT 1
+ OpExecutionMode %main OutputTrianglesEXT
+ OpDecorate %V SpecId 0
+ OpDecorate %P SpecId 1
+ OpDecorate %gl_MeshPerVertexEXT Block
+ OpMemberDecorate %gl_MeshPerVertexEXT 0 BuiltIn Position
+ OpMemberDecorate %gl_MeshPerVertexEXT 1 BuiltIn PointSize
+ OpMemberDecorate %gl_MeshPerVertexEXT 2 BuiltIn ClipDistance
+ OpMemberDecorate %gl_MeshPerVertexEXT 3 BuiltIn CullDistance
+ OpDecorate %gl_PrimitiveTriangleIndicesEXT BuiltIn PrimitiveTriangleIndicesEXT
+ %void = OpTypeVoid
+ %4 = OpTypeFunction %void
+ %uint = OpTypeInt 32 0
+ %uint_1 = OpConstant %uint 1
+ %V = OpSpecConstant %uint 500
+ %P = OpSpecConstant %uint 500
+ %float = OpTypeFloat 32
+ %v4float = OpTypeVector %float 4
+%_arr_float_uint_1 = OpTypeArray %float %uint_1
+%gl_MeshPerVertexEXT = OpTypeStruct %v4float %float %_arr_float_uint_1 %_arr_float_uint_1
+ %uint_3 = OpConstant %uint 3
+%_arr_gl_MeshPerVertexEXT_uint_3 = OpTypeArray %gl_MeshPerVertexEXT %uint_3
+%_ptr_Output__arr_gl_MeshPerVertexEXT_uint_3 = OpTypePointer Output %_arr_gl_MeshPerVertexEXT_uint_3
+%gl_MeshVerticesEXT = OpVariable %_ptr_Output__arr_gl_MeshPerVertexEXT_uint_3 Output
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+ %float_0 = OpConstant %float 0
+ %22 = OpConstantComposite %v4float %float_0 %float_0 %float_0 %float_0
+%_ptr_Output_v4float = OpTypePointer Output %v4float
+ %v3uint = OpTypeVector %uint 3
+%_arr_v3uint_uint_1 = OpTypeArray %v3uint %uint_1
+%_ptr_Output__arr_v3uint_uint_1 = OpTypePointer Output %_arr_v3uint_uint_1
+%gl_PrimitiveTriangleIndicesEXT = OpVariable %_ptr_Output__arr_v3uint_uint_1 Output
+ %uint_0 = OpConstant %uint 0
+ %uint_2 = OpConstant %uint 2
+ %31 = OpConstantComposite %v3uint %uint_0 %uint_1 %uint_2
+%_ptr_Output_v3uint = OpTypePointer Output %v3uint
+ %main = OpFunction %void None %4
+ %6 = OpLabel
+ OpSetMeshOutputsEXT %V %P
+ %24 = OpAccessChain %_ptr_Output_v4float %gl_MeshVerticesEXT %int_0 %int_0
+ OpStore %24 %22
+ %33 = OpAccessChain %_ptr_Output_v3uint %gl_PrimitiveTriangleIndicesEXT %int_0
+ OpStore %33 %31
+ OpReturn
+ OpFunctionEnd
+)";
+
+ CompileSuccessfully(body, SPV_ENV_VULKAN_1_3);
+ EXPECT_EQ(SPV_SUCCESS, ValidateInstructions(SPV_ENV_VULKAN_1_3));
+}
+
} // namespace
} // namespace val
} // namespace spvtools
