Skip to content

Commit

Permalink
layers: Applied feeback
Browse files Browse the repository at this point in the history
  • Loading branch information
spencer-lunarg committed Oct 1, 2024
1 parent a1492d4 commit f974303
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 11 deletions.
2 changes: 1 addition & 1 deletion docs/debug_printf.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ For those who "just need to quick use it" there is also a `set VK_LAYER_PRINTF_O

> All settings are found in Vulkan Configurator (`VkConfig`)
>
> Even if you use `VK_LAYER_PRINTF_ONLY_PRESET` you need to set this settings yourself as desired
> Even if you use `VK_LAYER_PRINTF_ONLY_PRESET` you need to set those settings yourself as desired
There are currently 3 environment variables that are used for settings in Debug Printf

Expand Down
7 changes: 3 additions & 4 deletions layers/gpu/instrumentation/gpu_shader_instrumentor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1371,21 +1371,20 @@ bool GpuShaderInstrumentor::InstrumentShader(const vvl::span<const uint32_t> &in

gpu::spirv::Module module(input_spirv, debug_report, module_settings);

GpuAVSettings::ShaderInstrumentation &shader_instrumentation = gpuav_settings.shader_instrumentation;
bool modified = false;

// If descriptor indexing is enabled, enable length checks and updated descriptor checks
if (shader_instrumentation.bindless_descriptor) {
if (gpuav_settings.shader_instrumentation.bindless_descriptor) {
modified |= module.RunPassBindlessDescriptor();
modified |= module.RunPassNonBindlessOOBBuffer();
modified |= module.RunPassNonBindlessOOBTexelBuffer();
}

if (shader_instrumentation.buffer_device_address) {
if (gpuav_settings.shader_instrumentation.buffer_device_address) {
modified |= module.RunPassBufferDeviceAddress();
}

if (shader_instrumentation.ray_query) {
if (gpuav_settings.shader_instrumentation.ray_query) {
modified |= module.RunPassRayQuery();
}

Expand Down
9 changes: 6 additions & 3 deletions layers/gpu/resources/gpuav_subclasses.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,13 @@ void CommandBuffer::PostProcess(VkQueue queue, const Location &loc) {
char *data;

VkResult result = vmaMapMemory(gpuav->vma_allocator_, buffer_info.output_mem_block.allocation, (void **)&data);
if (result == VK_SUCCESS) {
debug_printf::AnalyzeAndGenerateMessage(*gpuav, VkHandle(), queue, buffer_info, (uint32_t *)data, loc);
vmaUnmapMemory(gpuav->vma_allocator_, buffer_info.output_mem_block.allocation);
if (result != VK_SUCCESS) {
gpuav->InternalError(queue, loc, "Unable to map device memory allocated for DebugPrintf buffer.", true);
return;
}

debug_printf::AnalyzeAndGenerateMessage(*gpuav, VkHandle(), queue, buffer_info, (uint32_t *)data, loc);
vmaUnmapMemory(gpuav->vma_allocator_, buffer_info.output_mem_block.allocation);
}

// CommandBuffer::Destroy can happen on an other thread,
Expand Down
3 changes: 2 additions & 1 deletion layers/layer_options.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,8 @@ void ProcessConfigAndEnvSettings(ConfigAndEnvSettings *settings_data) {
VK_LAYER_VALIDATE_BEST_PRACTICES_NVIDIA);
SetValidationSetting(layer_setting_set, settings_data->enables, sync_validation, VK_LAYER_VALIDATE_SYNC);

// Before GPU-AV and DebugPrintf were megered, we used this enum to set GPU-AV and DebugPrintf in vkconfig.
// These were deprecated after the 1.3.296 SDK release
// Before GPU-AV and DebugPrintf were merged, we used this enum to set GPU-AV and DebugPrintf in vkconfig.
// This code should in theory be dead since removing it from vkconfig, but keep just incase for a bit
if (vkuHasLayerSetting(layer_setting_set, DEPRECATED_VK_LAYER_VALIDATE_GPU_BASED)) {
std::string setting_value;
Expand Down
3 changes: 1 addition & 2 deletions layers/layer_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ enum EnableFlags {
vendor_specific_amd,
vendor_specific_img,
vendor_specific_nvidia,
// DebugPrintf was merged with GPU-AV, so we now just use this funnel into the new settings
debug_printf_validation,
debug_printf_validation, // Will set GpuAVSettings::debug_printf_enabled underneath
sync_validation,
// Insert new enables above this line
kMaxEnableFlags,
Expand Down

0 comments on commit f974303

Please sign in to comment.