Skip to content

Commit

Permalink
gpu: Check index buffer contents in pre-draw validation
Browse files Browse the repository at this point in the history
Fixes #2492

Completely rework gpuav_draw, to make it easier to add validation,
and debug existing one.
When adding validation for some VkBuffer, the 2 more importants parts
are 1) creating the validation shader and 2) have an informative error
message. gpuav_draw.cpp has been redesigned around that idea.
Each shader is in charge of its small validation, no uber shader trying
to cater to all needs. Doing that ends up being a mess, with the need
to correctly pipe data to the validation shader.
Validation shaders are now more fine grained.
The C++ has been reworked around some template to streamline the
process of adding validation. The general idea is the following:
1) Declase a new "shader" struct representing validation shader
bindings and push contants
2) fill that struct
3) bind this struct and the validation pipeline
4) dispatch the appropriate number of draws

About 4): we used to dispatch one draw, and do for loop in the shader
to scan buffer elements. GPU are not good at that, instead a proper
amount of draws should be dispatched, each in charge of doing it's
small validation logic.
  • Loading branch information
jeremyg-lunarg authored and arno-lunarg committed Sep 4, 2024
1 parent 068a9dc commit a745204
Show file tree
Hide file tree
Showing 56 changed files with 4,715 additions and 1,312 deletions.
14 changes: 12 additions & 2 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,18 @@ vvl_sources = [
"layers/vulkan/generated/cmd_validation_copy_buffer_to_image_comp.h",
"layers/vulkan/generated/cmd_validation_dispatch_comp.cpp",
"layers/vulkan/generated/cmd_validation_dispatch_comp.h",
"layers/vulkan/generated/cmd_validation_draw_vert.cpp",
"layers/vulkan/generated/cmd_validation_draw_vert.h",
"layers/vulkan/generated/cmd_validation_count_buffer_vert.h",
"layers/vulkan/generated/cmd_validation_count_buffer_vert.cpp",
"layers/vulkan/generated/cmd_validation_first_instance_vert.h",
"layers/vulkan/generated/cmd_validation_first_instance_vert.cpp",
"layers/vulkan/generated/cmd_validation_draw_indexed_vert.h",
"layers/vulkan/generated/cmd_validation_draw_indexed_vert.cpp",
"layers/vulkan/generated/cmd_validation_draw_indexed_indirect_index_buffer_vert.h",
"layers/vulkan/generated/cmd_validation_draw_indexed_indirect_index_buffer_vert.cpp",
"layers/vulkan/generated/cmd_validation_draw_indexed_indirect_vertex_buffer_vert.h",
"layers/vulkan/generated/cmd_validation_draw_indexed_indirect_vertex_buffer_vert.cpp",
"layers/vulkan/generated/cmd_validation_draw_mesh_indirect_vert.h",
"layers/vulkan/generated/cmd_validation_draw_mesh_indirect_vert.cpp",
"layers/vulkan/generated/cmd_validation_trace_rays_rgen.cpp",
"layers/vulkan/generated/cmd_validation_trace_rays_rgen.h",
"layers/vulkan/generated/command_validation.cpp",
Expand Down
14 changes: 12 additions & 2 deletions layers/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,18 @@ target_sources(vvl PRIVATE
${API_TYPE}/generated/cmd_validation_copy_buffer_to_image_comp.cpp
${API_TYPE}/generated/cmd_validation_dispatch_comp.h
${API_TYPE}/generated/cmd_validation_dispatch_comp.cpp
${API_TYPE}/generated/cmd_validation_draw_vert.h
${API_TYPE}/generated/cmd_validation_draw_vert.cpp
${API_TYPE}/generated/cmd_validation_count_buffer_vert.h
${API_TYPE}/generated/cmd_validation_count_buffer_vert.cpp
${API_TYPE}/generated/cmd_validation_first_instance_vert.h
${API_TYPE}/generated/cmd_validation_first_instance_vert.cpp
${API_TYPE}/generated/cmd_validation_draw_indexed_vert.h
${API_TYPE}/generated/cmd_validation_draw_indexed_vert.cpp
${API_TYPE}/generated/cmd_validation_draw_indexed_indirect_index_buffer_vert.h
${API_TYPE}/generated/cmd_validation_draw_indexed_indirect_index_buffer_vert.cpp
${API_TYPE}/generated/cmd_validation_draw_indexed_indirect_vertex_buffer_vert.h
${API_TYPE}/generated/cmd_validation_draw_indexed_indirect_vertex_buffer_vert.cpp
${API_TYPE}/generated/cmd_validation_draw_mesh_indirect_vert.h
${API_TYPE}/generated/cmd_validation_draw_mesh_indirect_vert.cpp
${API_TYPE}/generated/cmd_validation_trace_rays_rgen.h
${API_TYPE}/generated/cmd_validation_trace_rays_rgen.cpp
gpu/core/gpu_settings.h
Expand Down
4 changes: 2 additions & 2 deletions layers/gpu/cmd_validation/gpuav_cmd_validation_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

namespace gpuav {

void BindValidationCmdsCommonDescSet(Validator &gpuav, CommandBuffer &cb_state, VkPipelineBindPoint bind_point,
VkPipelineLayout pipeline_layout, uint32_t cmd_index, uint32_t error_logger_index) {
void BindErrorLoggingDescriptorSet(Validator &gpuav, CommandBuffer &cb_state, VkPipelineBindPoint bind_point,
VkPipelineLayout pipeline_layout, uint32_t cmd_index, uint32_t error_logger_index) {
assert(cmd_index < cst::indices_count);
assert(error_logger_index < cst::indices_count);
std::array<uint32_t, 2> dynamic_offsets = {
Expand Down
4 changes: 2 additions & 2 deletions layers/gpu/cmd_validation/gpuav_cmd_validation_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class RestorablePipelineState {
std::vector<vvl::ShaderObject*> shader_objects_;
};

void BindValidationCmdsCommonDescSet(Validator& gpuav, CommandBuffer& cb_state, VkPipelineBindPoint bind_point,
VkPipelineLayout pipeline_layout, uint32_t cmd_index, uint32_t error_logger_index);
void BindErrorLoggingDescriptorSet(Validator& gpuav, CommandBuffer& cb_state, VkPipelineBindPoint bind_point,
VkPipelineLayout pipeline_layout, uint32_t cmd_index, uint32_t error_logger_index);

} // namespace gpuav
5 changes: 2 additions & 3 deletions layers/gpu/cmd_validation/gpuav_copy_buffer_to_image.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,8 @@ void InsertCopyBufferToImageValidation(Validator &gpuav, const Location &loc, Co
// Insert diagnostic dispatch
DispatchCmdBindPipeline(cb_state.VkHandle(), VK_PIPELINE_BIND_POINT_COMPUTE, shared_copy_validation_resources.pipeline);

BindValidationCmdsCommonDescSet(gpuav, cb_state, VK_PIPELINE_BIND_POINT_COMPUTE,
shared_copy_validation_resources.pipeline_layout, 0,
static_cast<uint32_t>(cb_state.per_command_error_loggers.size()));
BindErrorLoggingDescriptorSet(gpuav, cb_state, VK_PIPELINE_BIND_POINT_COMPUTE, shared_copy_validation_resources.pipeline_layout,
0, static_cast<uint32_t>(cb_state.per_command_error_loggers.size()));
DispatchCmdBindDescriptorSets(cb_state.VkHandle(), VK_PIPELINE_BIND_POINT_COMPUTE,
shared_copy_validation_resources.pipeline_layout, glsl::kDiagPerCmdDescriptorSet, 1,
&validation_desc_set, 0, nullptr);
Expand Down
4 changes: 2 additions & 2 deletions layers/gpu/cmd_validation/gpuav_dispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ void InsertIndirectDispatchValidation(Validator &gpuav, const Location &loc, Com
push_constants[3] = static_cast<uint32_t>((indirect_offset / sizeof(uint32_t)));
DispatchCmdPushConstants(cb_state.VkHandle(), shared_dispatch_resources.pipeline_layout, VK_SHADER_STAGE_COMPUTE_BIT, 0,
sizeof(push_constants), push_constants);
BindValidationCmdsCommonDescSet(gpuav, cb_state, VK_PIPELINE_BIND_POINT_COMPUTE, shared_dispatch_resources.pipeline_layout,
cb_state.compute_index, static_cast<uint32_t>(cb_state.per_command_error_loggers.size()));
BindErrorLoggingDescriptorSet(gpuav, cb_state, VK_PIPELINE_BIND_POINT_COMPUTE, shared_dispatch_resources.pipeline_layout,
cb_state.compute_index, static_cast<uint32_t>(cb_state.per_command_error_loggers.size()));
DispatchCmdBindDescriptorSets(cb_state.VkHandle(), VK_PIPELINE_BIND_POINT_COMPUTE, shared_dispatch_resources.pipeline_layout,
glsl::kDiagPerCmdDescriptorSet, 1, &indirect_buffer_desc_set, 0, nullptr);
DispatchCmdDispatch(cb_state.VkHandle(), 1, 1, 1);
Expand Down
Loading

0 comments on commit a745204

Please sign in to comment.