Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gpuav: Keep instruction count to make error message faster #9381

Closed
wants to merge 1 commit into from
Closed

gpuav: Keep instruction count to make error message faster #9381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions layers/gpuav/debug_printf/debug_printf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Instruction> 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);
Expand Down
5 changes: 3 additions & 2 deletions layers/gpuav/instrumentation/gpuav_instrumentation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Instruction> 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);
Expand Down
34 changes: 26 additions & 8 deletions layers/gpuav/instrumentation/gpuav_shader_instrumentor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,13 +355,21 @@ void GpuShaderInstrumentor::PostCallRecordCreateShadersEXT(VkDevice device, uint
if (!instrumentation_data.IsInstrumented()) {
continue;
}
if (const auto &shader_object_state = Get<vvl::ShaderObject>(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<vvl::ShaderObject>(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<uint32_t> 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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was just straight wrong, we need the original for the Debug Info to work well, we are lacking any tests in gpu_av_shader_debug_info.cpp for shader object to have caught this

pShaders[i], std::move(code), instruction_count);
}
}

Expand Down Expand Up @@ -939,21 +947,26 @@ 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<uint32_t> 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) {
shader_module_handle = kPipelineStageInfoHandle;
}

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);
}
}

Expand Down Expand Up @@ -1112,21 +1125,26 @@ 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<uint32_t> 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) {
shader_module_handle = kPipelineStageInfoHandle;
}

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);
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion layers/gpuav/instrumentation/gpuav_shader_instrumentor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ struct InstrumentedShader {
VkPipeline pipeline;
VkShaderModule shader_module;
VkShaderEXT shader_object;
std::vector<uint32_t> instrumented_spirv;
// Keep original SPIR-V to map bad Shader Debug Info line numbers
std::vector<uint32_t> original_spirv;
// we will need to turn the vector<uint32_t> back into an vector<Instruction> 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.
Expand Down
7 changes: 6 additions & 1 deletion layers/state_tracker/shader_instruction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<const uint32_t>& spirv, std::vector<Instruction>& instructions) {
void GenerateInstructions(const vvl::span<const uint32_t>& spirv, uint32_t instruction_count,
std::vector<Instruction>& instructions) {
// Need to use until we have native std::span in c++20
using spirv_iterator = vvl::enumeration<const uint32_t, const uint32_t*>::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);
Expand Down
3 changes: 2 additions & 1 deletion layers/state_tracker/shader_instruction.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class Instruction {
#endif
};

void GenerateInstructions(const vvl::span<const uint32_t>& spirv, std::vector<Instruction>& instructions);
void GenerateInstructions(const vvl::span<const uint32_t>& spirv, uint32_t instruction_count,
std::vector<Instruction>& instructions);

} // namespace spirv
2 changes: 2 additions & 0 deletions layers/state_tracker/shader_module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down