diff --git a/layers/gpuav/debug_printf/debug_printf.cpp b/layers/gpuav/debug_printf/debug_printf.cpp index cefdf64b6fa..c37ad09a66f 100644 --- a/layers/gpuav/debug_printf/debug_printf.cpp +++ b/layers/gpuav/debug_printf/debug_printf.cpp @@ -210,13 +210,13 @@ void AnalyzeAndGenerateMessage(Validator &gpuav, VkCommandBuffer command_buffer, } // without the instrumented spirv, there is nothing valuable to print out - if (!instrumented_shader || instrumented_shader->instrumented_spirv.empty()) { + if (!instrumented_shader || instrumented_shader->original_spirv.empty()) { gpuav.InternalWarning(queue, loc, "Can't find instructions from any handles in shader_map"); return; } std::vector instructions; - ::spirv::GenerateInstructions(instrumented_shader->instrumented_spirv, instructions); + ::spirv::GenerateInstructions(instrumented_shader->original_spirv, instrumented_shader->instruction_count, instructions); // Search through the shader source for the printf format string for this invocation std::string format_string = FindFormatString(instructions, debug_record->format_string_id); diff --git a/layers/gpuav/instrumentation/gpuav_instrumentation.cpp b/layers/gpuav/instrumentation/gpuav_instrumentation.cpp index 5b4a3362af9..726d0ccb9e1 100644 --- a/layers/gpuav/instrumentation/gpuav_instrumentation.cpp +++ b/layers/gpuav/instrumentation/gpuav_instrumentation.cpp @@ -1062,8 +1062,9 @@ bool LogInstrumentationError(Validator &gpuav, const CommandBuffer &cb_state, co // If we somehow can't find our state, we can still report our error message std::vector instructions; - if (instrumented_shader && !instrumented_shader->instrumented_spirv.empty()) { - ::spirv::GenerateInstructions(instrumented_shader->instrumented_spirv, instructions); + if (instrumented_shader && !instrumented_shader->original_spirv.empty()) { + ::spirv::GenerateInstructions(instrumented_shader->original_spirv, instrumented_shader->instruction_count, + instructions); } std::string debug_region_name = cb_state.GetDebugLabelRegion(label_command_i, initial_label_stack); Location loc_with_debug_region(loc, debug_region_name); diff --git a/layers/gpuav/instrumentation/gpuav_shader_instrumentor.cpp b/layers/gpuav/instrumentation/gpuav_shader_instrumentor.cpp index c36df6c21ba..befa0429591 100644 --- a/layers/gpuav/instrumentation/gpuav_shader_instrumentor.cpp +++ b/layers/gpuav/instrumentation/gpuav_shader_instrumentor.cpp @@ -355,13 +355,21 @@ void GpuShaderInstrumentor::PostCallRecordCreateShadersEXT(VkDevice device, uint if (!instrumentation_data.IsInstrumented()) { continue; } - if (const auto &shader_object_state = Get(pShaders[i])) { - shader_object_state->instrumentation_data.was_instrumented = true; - shader_object_state->instrumentation_data.unique_shader_id = instrumentation_data.unique_shader_id; + const auto &shader_object_state = Get(pShaders[i]); + ASSERT_AND_CONTINUE(shader_object_state); + + shader_object_state->instrumentation_data.was_instrumented = true; + shader_object_state->instrumentation_data.unique_shader_id = instrumentation_data.unique_shader_id; + + uint32_t instruction_count = 0; + std::vector code; + if (shader_object_state->spirv) { + instruction_count = shader_object_state->spirv->InstructionCount(); + code = shader_object_state->spirv->words_; } instrumented_shaders_map_.insert_or_assign(instrumentation_data.unique_shader_id, VK_NULL_HANDLE, VK_NULL_HANDLE, - pShaders[i], instrumentation_data.instrumented_spirv); + pShaders[i], std::move(code), instruction_count); } } @@ -939,13 +947,18 @@ void GpuShaderInstrumentor::PostCallRecordPipelineCreationShaderInstrumentation( const auto &stage_state = pipeline_state.stage_states[stage_state_i]; auto &module_state = stage_state.module_state; + uint32_t instruction_count = 0; + // We currently need to store a copy of the original, non-instrumented shader so if there is debug information, // we can reference it by the instruction number printed out in the shader. Since the application can destroy the // original VkShaderModule, there is a chance this will be gone, we need to copy it now. // TODO - in the instrumentation, instead of printing the instruction number only, if we print out debug info, we // can remove this copy std::vector code; - if (module_state && module_state->spirv) code = module_state->spirv->words_; + if (module_state && module_state->spirv) { + code = module_state->spirv->words_; + instruction_count = module_state->spirv->InstructionCount(); + } VkShaderModule shader_module_handle = module_state->VkHandle(); if (shader_module_handle == VK_NULL_HANDLE && instrumentation_metadata.passed_in_shader_stage_ci) { @@ -953,7 +966,7 @@ void GpuShaderInstrumentor::PostCallRecordPipelineCreationShaderInstrumentation( } instrumented_shaders_map_.insert_or_assign(instrumentation_metadata.unique_shader_id, pipeline_state.VkHandle(), - shader_module_handle, VK_NULL_HANDLE, std::move(code)); + shader_module_handle, VK_NULL_HANDLE, std::move(code), instruction_count); } } @@ -1112,13 +1125,18 @@ void GpuShaderInstrumentor::PostCallRecordPipelineCreationShaderInstrumentationG const auto &stage_state = lib->stage_states[stage_state_i]; auto &module_state = stage_state.module_state; + uint32_t instruction_count = 0; + // We currently need to store a copy of the original, non-instrumented shader so if there is debug information, // we can reference it by the instruction number printed out in the shader. Since the application can destroy the // original VkShaderModule, there is a chance this will be gone, we need to copy it now. // TODO - in the instrumentation, instead of printing the instruction number only, if we print out debug info, we // can remove this copy std::vector code; - if (module_state && module_state->spirv) code = module_state->spirv->words_; + if (module_state && module_state->spirv) { + code = module_state->spirv->words_; + instruction_count = module_state->spirv->InstructionCount(); + } VkShaderModule shader_module_handle = module_state->VkHandle(); if (shader_module_handle == VK_NULL_HANDLE && instrumentation_metadata.passed_in_shader_stage_ci) { @@ -1126,7 +1144,7 @@ void GpuShaderInstrumentor::PostCallRecordPipelineCreationShaderInstrumentationG } instrumented_shaders_map_.insert_or_assign(instrumentation_metadata.unique_shader_id, lib->VkHandle(), - shader_module_handle, VK_NULL_HANDLE, std::move(code)); + shader_module_handle, VK_NULL_HANDLE, std::move(code), instruction_count); } } } diff --git a/layers/gpuav/instrumentation/gpuav_shader_instrumentor.h b/layers/gpuav/instrumentation/gpuav_shader_instrumentor.h index 567f962a846..b91a9614d95 100644 --- a/layers/gpuav/instrumentation/gpuav_shader_instrumentor.h +++ b/layers/gpuav/instrumentation/gpuav_shader_instrumentor.h @@ -58,7 +58,10 @@ struct InstrumentedShader { VkPipeline pipeline; VkShaderModule shader_module; VkShaderEXT shader_object; - std::vector instrumented_spirv; + // Keep original SPIR-V to map bad Shader Debug Info line numbers + std::vector original_spirv; + // we will need to turn the vector back into an vector and faster to know how many to reserve + uint32_t instruction_count; }; // Historically this was an common interface to both GPU-AV and DebugPrintf before the were merged together. diff --git a/layers/state_tracker/shader_instruction.cpp b/layers/state_tracker/shader_instruction.cpp index c73bb838f2c..c1dd6d6e308 100644 --- a/layers/state_tracker/shader_instruction.cpp +++ b/layers/state_tracker/shader_instruction.cpp @@ -189,11 +189,16 @@ spv::StorageClass Instruction::StorageClass() const { // When logging from things such as GPU-AV, we need to do some SPIR-V processing, but is just to inspect single instructions without // knowledge of the rest of the module. This function just turns the saved vector of uint32_t into the Instruction class to make it // easier to use. -void GenerateInstructions(const vvl::span& spirv, std::vector& instructions) { +void GenerateInstructions(const vvl::span& spirv, uint32_t instruction_count, + std::vector& instructions) { // Need to use until we have native std::span in c++20 using spirv_iterator = vvl::enumeration::iterator; assert(instructions.empty()); + + // profiling showed this is a huge bottleneck to not have this reserved + instructions.reserve(instruction_count); + spirv_iterator it = spirv.begin(); it += 5; // skip first 5 word of header instructions.reserve(spirv.size() * 4); diff --git a/layers/state_tracker/shader_instruction.h b/layers/state_tracker/shader_instruction.h index bd9be45f2b1..59fccccda01 100644 --- a/layers/state_tracker/shader_instruction.h +++ b/layers/state_tracker/shader_instruction.h @@ -104,6 +104,7 @@ class Instruction { #endif }; -void GenerateInstructions(const vvl::span& spirv, std::vector& instructions); +void GenerateInstructions(const vvl::span& spirv, uint32_t instruction_count, + std::vector& instructions); } // namespace spirv diff --git a/layers/state_tracker/shader_module.h b/layers/state_tracker/shader_module.h index 36e1d55c0cb..ccb521bb56f 100644 --- a/layers/state_tracker/shader_module.h +++ b/layers/state_tracker/shader_module.h @@ -747,6 +747,8 @@ struct Module { return std::any_of(static_data_.capability_list.begin(), static_data_.capability_list.end(), [find_capability](const spv::Capability &capability) { return capability == find_capability; }); } + + uint32_t InstructionCount() const { return (uint32_t)static_data_.instructions.size(); } }; } // namespace spirv