From 0059ce1b614a3da218ff28cb47c0c4e43444c591 Mon Sep 17 00:00:00 2001 From: spencer-lunarg Date: Mon, 17 Jun 2024 13:42:45 -0500 Subject: [PATCH] gpu:: Add support when shader is inline in pNext --- layers/chassis/chassis_modification_state.h | 6 +- layers/core_checks/cc_pipeline_graphics.cpp | 2 +- layers/core_checks/cc_spirv.cpp | 2 + layers/gpu/debug_printf/debug_printf.cpp | 8 +- .../gpu/error_message/gpuav_error_message.cpp | 50 ++++--- .../gpu_shader_instrumentor.cpp | 132 ++++++++++-------- .../instrumentation/gpu_shader_instrumentor.h | 7 +- layers/state_tracker/pipeline_sub_state.cpp | 4 +- layers/state_tracker/shader_module.h | 2 +- tests/unit/debug_printf.cpp | 3 +- tests/unit/gpu_av_buffer_device_address.cpp | 94 +++++++++++++ 11 files changed, 220 insertions(+), 90 deletions(-) diff --git a/layers/chassis/chassis_modification_state.h b/layers/chassis/chassis_modification_state.h index 0ffbda1a909..58403560f4c 100644 --- a/layers/chassis/chassis_modification_state.h +++ b/layers/chassis/chassis_modification_state.h @@ -82,7 +82,8 @@ struct CreateGraphicsPipelines { std::vector shader_unique_id_maps; const VkGraphicsPipelineCreateInfo* pCreateInfos; // Used to if VkShaderModuleCreateInfo is passed down VkPipelineShaderStageCreateInfo - spirv::StatelessData stateless_data[spirv::kCommonMaxShaderStages]; + bool passed_in_shader_stage_ci; + spirv::StatelessData stateless_data[spirv::kCommonMaxGraphicsShaderStages]; CreateGraphicsPipelines(const VkGraphicsPipelineCreateInfo* create_info) { pCreateInfos = create_info; } }; @@ -92,6 +93,7 @@ struct CreateComputePipelines { std::vector shader_unique_id_maps; // not used, here for template function const VkComputePipelineCreateInfo* pCreateInfos; // Used to if VkShaderModuleCreateInfo is passed down VkPipelineShaderStageCreateInfo + bool passed_in_shader_stage_ci; spirv::StatelessData stateless_data; CreateComputePipelines(const VkComputePipelineCreateInfo* create_info) { pCreateInfos = create_info; } @@ -101,6 +103,7 @@ struct CreateRayTracingPipelinesNV { std::vector modified_create_infos; std::vector shader_unique_id_maps; // not used, here for template function const VkRayTracingPipelineCreateInfoNV* pCreateInfos; + bool passed_in_shader_stage_ci; CreateRayTracingPipelinesNV(const VkRayTracingPipelineCreateInfoNV* create_info) { pCreateInfos = create_info; } }; @@ -109,6 +112,7 @@ struct CreateRayTracingPipelinesKHR { std::vector modified_create_infos; std::vector shader_unique_id_maps; // not used, here for template function const VkRayTracingPipelineCreateInfoKHR* pCreateInfos; + bool passed_in_shader_stage_ci; CreateRayTracingPipelinesKHR(const VkRayTracingPipelineCreateInfoKHR* create_info) { pCreateInfos = create_info; } }; diff --git a/layers/core_checks/cc_pipeline_graphics.cpp b/layers/core_checks/cc_pipeline_graphics.cpp index b439d5302fc..adfa63848dc 100644 --- a/layers/core_checks/cc_pipeline_graphics.cpp +++ b/layers/core_checks/cc_pipeline_graphics.cpp @@ -53,7 +53,7 @@ bool CoreChecks::PreCallValidateCreateGraphicsPipelines(VkDevice device, VkPipel // at state tracking time, and we state track pipelines first) if (i == 0) { for (uint32_t stage = 0; stage < pCreateInfos[0].stageCount; stage++) { - if (chassis_state.stateless_data[stage].pipeline_pnext_module) { + if (stage < spirv::kCommonMaxGraphicsShaderStages && chassis_state.stateless_data[stage].pipeline_pnext_module) { skip |= ValidateSpirvStateless( *chassis_state.stateless_data[stage].pipeline_pnext_module, chassis_state.stateless_data[stage], create_info_loc.dot(Field::pStages, stage).pNext(Struct::VkShaderModuleCreateInfo, Field::pCode)); diff --git a/layers/core_checks/cc_spirv.cpp b/layers/core_checks/cc_spirv.cpp index 3b5fc3662c0..e7d36d6f359 100644 --- a/layers/core_checks/cc_spirv.cpp +++ b/layers/core_checks/cc_spirv.cpp @@ -2875,6 +2875,8 @@ bool CoreChecks::ValidateEmitMeshTasksSize(const spirv::Module &module_state, co bool CoreChecks::ValidateSpirvStateless(const spirv::Module &module_state, const spirv::StatelessData &stateless_data, const Location &loc) const { bool skip = false; + if (!module_state.valid_spirv) return skip; + skip |= ValidateShaderClock(module_state, stateless_data, loc); skip |= ValidateAtomicsTypes(module_state, stateless_data, loc); skip |= ValidateVariables(module_state, loc); diff --git a/layers/gpu/debug_printf/debug_printf.cpp b/layers/gpu/debug_printf/debug_printf.cpp index 662ce9c620a..e72f5745357 100644 --- a/layers/gpu/debug_printf/debug_printf.cpp +++ b/layers/gpu/debug_printf/debug_printf.cpp @@ -310,7 +310,7 @@ void Validator::AnalyzeAndGenerateMessage(VkCommandBuffer command_buffer, VkQueu shader_object_handle = it->second.shader_object; instrumented_spirv = it->second.instrumented_spirv; } - assert(instrumented_spirv.size() != 0); + ASSERT_AND_RETURN(!instrumented_spirv.empty()); std::vector instructions; spirv::GenerateInstructions(instrumented_spirv, instructions); @@ -387,10 +387,10 @@ void Validator::AnalyzeAndGenerateMessage(VkCommandBuffer command_buffer, VkQueu common_message); UtilGenerateSourceMessages(instructions, &debug_output_buffer[index], true, filename_message, source_message); if (use_stdout) { - std::cout << "WARNING-DEBUG-PRINTF " << common_message.c_str() << " " << shader_message.str().c_str() << " " - << filename_message.c_str() << " " << source_message.c_str(); + std::cout << "WARNING-DEBUG-PRINTF " << shader_message.str().c_str() << "\n" + << common_message.c_str() << " " << filename_message.c_str() << " " << source_message.c_str(); } else { - LogInfo("WARNING-DEBUG-PRINTF", queue, loc, "%s %s %s%s", common_message.c_str(), shader_message.str().c_str(), + LogInfo("WARNING-DEBUG-PRINTF", queue, loc, "%s\n%s%s%s", shader_message.str().c_str(), common_message.c_str(), filename_message.c_str(), source_message.c_str()); } } else { diff --git a/layers/gpu/error_message/gpuav_error_message.cpp b/layers/gpu/error_message/gpuav_error_message.cpp index 4e451c7dda2..264bd748177 100644 --- a/layers/gpu/error_message/gpuav_error_message.cpp +++ b/layers/gpu/error_message/gpuav_error_message.cpp @@ -141,13 +141,14 @@ void UtilGenerateCommonMessage(const DebugReport *debug_report, const VkCommandB std::unique_lock lock(debug_report->debug_output_mutex); strm << std::hex << std::showbase << "Internal Error: Unable to locate information for shader used in command buffer " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(commandBuffer)) << "(" << HandleToUint64(commandBuffer) - << "). "; + << ")\n"; assert(true); } else { std::unique_lock lock(debug_report->debug_output_mutex); strm << std::hex << std::showbase << "Command buffer " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(commandBuffer)) << "(" << HandleToUint64(commandBuffer) - << "). "; + << ")\n"; + if (pipeline_bind_point == VK_PIPELINE_BIND_POINT_GRAPHICS) { strm << "Draw "; } else if (pipeline_bind_point == VK_PIPELINE_BIND_POINT_COMPUTE) { @@ -158,20 +159,24 @@ void UtilGenerateCommonMessage(const DebugReport *debug_report, const VkCommandB assert(false); strm << "Unknown Pipeline Operation "; } - if (shader_module_handle) { - strm << "Index " << operation_index << ". " - << "Pipeline " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(pipeline_handle)) << "(" - << HandleToUint64(pipeline_handle) << "). " - << "Shader Module " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(shader_module_handle)) << "(" - << HandleToUint64(shader_module_handle) << "). "; + + strm << "Index " << operation_index << "\n"; + if (shader_module_handle == VK_NULL_HANDLE) { + strm << "Shader Object " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(shader_object_handle)) << "(" + << HandleToUint64(shader_object_handle) << ")\n"; } else { - strm << "Index " << operation_index << ". " - << "Shader Object " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(shader_object_handle)) << "(" - << HandleToUint64(shader_object_handle) << "). "; + strm << "Pipeline " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(pipeline_handle)) << "(" + << HandleToUint64(pipeline_handle) << ")\n"; + if (shader_module_handle == gpu::kPipelineStageInfoHandle) { + strm << "Shader Module was passed in via VkPipelineShaderStageCreateInfo::pNext\n"; + } else { + strm << "Shader Module " << LookupDebugUtilsNameNoLock(debug_report, HandleToUint64(shader_module_handle)) << "(" + << HandleToUint64(shader_module_handle) << ")\n"; + } } } strm << std::dec << std::noshowbase; - strm << "Shader Instruction Index = " << debug_record[gpuav::glsl::kHeaderInstructionIdOffset] << ". "; + strm << "Shader Instruction Index = " << debug_record[gpuav::glsl::kHeaderInstructionIdOffset] << "\n"; msg = strm.str(); } @@ -281,7 +286,7 @@ void UtilGenerateSourceMessages(const std::vector &instructi std::string reported_filename; if (reported_file_id == 0) { filename_stream - << "Unable to find SPIR-V OpLine for source information. Build shader with debug info to get source information."; + << "Unable to find SPIR-V OpLine for source information. Build shader with debug info to get source information.\n"; } else { bool found_opstring = false; std::string prefix; @@ -303,16 +308,15 @@ void UtilGenerateSourceMessages(const std::vector &instructi if (reported_column_number > 0) { filename_stream << ", column " << reported_column_number; } - filename_stream << "."; + filename_stream << "\n"; break; } } if (!found_opstring) { - filename_stream << "Unable to find SPIR-V OpString for file id " << reported_file_id << " from OpLine instruction." - << std::endl; + filename_stream << "Unable to find SPIR-V OpString for file id " << reported_file_id << " from OpLine instruction.\n"; filename_stream << "File ID = " << reported_file_id << ", Line Number = " << reported_line_number - << ", Column = " << reported_column_number << std::endl; + << ", Column = " << reported_column_number << "\n"; } } filename_msg = filename_stream.str(); @@ -355,16 +359,16 @@ void UtilGenerateSourceMessages(const std::vector &instructi std::vector::size_type opsource_index = (reported_line_number - saved_line_number) + 1 + saved_opsource_offset; if (opsource_index < opsource_lines.size()) { - source_stream << "\n" << reported_line_number << ": " << opsource_lines[opsource_index].c_str(); + source_stream << "\n" << reported_line_number << ": " << opsource_lines[opsource_index].c_str() << "\n"; } else { source_stream << "Internal error: calculated source line of " << opsource_index << " for source size of " - << opsource_lines.size() << " lines."; + << opsource_lines.size() << " lines\n"; } } else { - source_stream << "Unable to find suitable #line directive in SPIR-V OpSource."; + source_stream << "Unable to find suitable #line directive in SPIR-V OpSource\n"; } } else { - source_stream << "Unable to find SPIR-V OpSource."; + source_stream << "Unable to find SPIR-V OpSource\n"; } } source_msg = source_stream.str(); @@ -613,11 +617,11 @@ bool Validator::AnalyzeAndGenerateMessage(VkCommandBuffer cmd_buffer, const LogO if (uses_robustness && oob_access) { if (gpuav_settings.warn_on_robust_oob) { - LogWarning(vuid_msg.c_str(), objlist, loc, "%s %s %s %s%s", error_msg.c_str(), common_message.c_str(), + LogWarning(vuid_msg.c_str(), objlist, loc, "%s\n%s%s%s%s", error_msg.c_str(), common_message.c_str(), stage_message.c_str(), filename_message.c_str(), source_message.c_str()); } } else { - LogError(vuid_msg.c_str(), objlist, loc, "%s %s %s %s%s", error_msg.c_str(), common_message.c_str(), + LogError(vuid_msg.c_str(), objlist, loc, "%s\n%s%s%s%s", error_msg.c_str(), common_message.c_str(), stage_message.c_str(), filename_message.c_str(), source_message.c_str()); } } diff --git a/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp b/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp index 756539d3db5..3bf95477ed2 100644 --- a/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp +++ b/layers/gpu/instrumentation/gpu_shader_instrumentor.cpp @@ -540,7 +540,8 @@ void GpuShaderInstrumentor::PostCallRecordCreateGraphicsPipelines(VkDevice devic BaseClass::PostCallRecordCreateGraphicsPipelines(device, pipelineCache, count, pCreateInfos, pAllocator, pPipelines, record_obj, pipeline_states, chassis_state); UtilCopyCreatePipelineFeedbackData(count, pCreateInfos, chassis_state.modified_create_infos.data()); - PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data()); + PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data(), + chassis_state.passed_in_shader_stage_ci); } void GpuShaderInstrumentor::PostCallRecordCreateComputePipelines(VkDevice device, VkPipelineCache pipelineCache, uint32_t count, @@ -551,7 +552,8 @@ void GpuShaderInstrumentor::PostCallRecordCreateComputePipelines(VkDevice device BaseClass::PostCallRecordCreateComputePipelines(device, pipelineCache, count, pCreateInfos, pAllocator, pPipelines, record_obj, pipeline_states, chassis_state); UtilCopyCreatePipelineFeedbackData(count, pCreateInfos, chassis_state.modified_create_infos.data()); - PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data()); + PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data(), + chassis_state.passed_in_shader_stage_ci); } void GpuShaderInstrumentor::PostCallRecordCreateRayTracingPipelinesNV( @@ -561,7 +563,8 @@ void GpuShaderInstrumentor::PostCallRecordCreateRayTracingPipelinesNV( BaseClass::PostCallRecordCreateRayTracingPipelinesNV(device, pipelineCache, count, pCreateInfos, pAllocator, pPipelines, record_obj, pipeline_states, chassis_state); UtilCopyCreatePipelineFeedbackData(count, pCreateInfos, chassis_state.modified_create_infos.data()); - PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data()); + PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data(), + chassis_state.passed_in_shader_stage_ci); } void GpuShaderInstrumentor::PostCallRecordCreateRayTracingPipelinesKHR( @@ -571,7 +574,8 @@ void GpuShaderInstrumentor::PostCallRecordCreateRayTracingPipelinesKHR( BaseClass::PostCallRecordCreateRayTracingPipelinesKHR(device, deferredOperation, pipelineCache, count, pCreateInfos, pAllocator, pPipelines, record_obj, pipeline_states, chassis_state); UtilCopyCreatePipelineFeedbackData(count, pCreateInfos, chassis_state.modified_create_infos.data()); - PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data()); + PostCallRecordPipelineCreations(count, pCreateInfos, pAllocator, pPipelines, chassis_state.modified_create_infos.data(), + chassis_state.passed_in_shader_stage_ci); } // Remove all the shader trackers associated with this destroyed pipeline. @@ -652,6 +656,7 @@ void GpuShaderInstrumentor::PreCallRecordPipelineCreations(uint32_t count, const PipelineStates &pipeline_states, std::vector *new_pipeline_create_infos, const RecordObject &record_obj, ChassisState &chassis_state) { + chassis_state.passed_in_shader_stage_ci = false; // Walk through all the pipelines, make a copy of each and flag each pipeline that contains a shader that uses the debug // descriptor set index. for (uint32_t pipeline = 0; pipeline < count; ++pipeline) { @@ -689,57 +694,69 @@ void GpuShaderInstrumentor::PreCallRecordPipelineCreations(uint32_t count, const } else { // !replace_shaders implies that the instrumented shaders should be used. However, if this is a non-executable pipeline // library created with pre-raster or fragment shader state, it contains shaders that have not yet been instrumented - if (!pipe->HasFullState() && (pipe->pre_raster_state || pipe->fragment_shader_state)) { - for (const auto &stage_state : pipe->stage_states) { - auto module_state = std::const_pointer_cast(stage_state.module_state); - if (!module_state->Handle()) { - // If the shader module's handle is non-null, then it was defined with CreateShaderModule and covered by the - // case above. Otherwise, it is being defined during CGPL time - if (chassis_state.shader_unique_id_maps.size() <= pipeline) { - chassis_state.shader_unique_id_maps.resize(pipeline + 1); - } - const VkShaderStageFlagBits stage = stage_state.GetStage(); - // Now find the corresponding VkShaderModuleCreateInfo - auto &stage_ci = - GetShaderStageCI(new_pipeline_ci, stage); - // We're modifying the copied, safe create info, which is ok to be non-const - auto sm_ci = const_cast( - reinterpret_cast( - vku::FindStructInPNextChain(stage_ci.pNext))); - if (gpuav_settings.select_instrumented_shaders && sm_ci && !CheckForGpuAvEnabled(sm_ci->pNext)) continue; - uint32_t unique_shader_id = 0; - bool cached = false; - bool pass = false; - std::vector instrumented_spirv; - if (gpuav_settings.cache_instrumented_shaders) { - unique_shader_id = - hash_util::ShaderHash(module_state->spirv->words_.data(), module_state->spirv->words_.size()); - if (const auto spirv = instrumented_shaders_cache_.Get(unique_shader_id)) { - instrumented_spirv = *spirv; - cached = true; - } - } else { - unique_shader_id = unique_shader_module_id_++; - } - if (!cached) { - pass = InstrumentShader(module_state->spirv->words_, unique_shader_id, record_obj.location, - instrumented_spirv); - } - if (cached || pass) { - module_state->gpu_validation_shader_id = unique_shader_id; - // Now we need to update the shader code in VkShaderModuleCreateInfo - // module_state->Handle() == VK_NULL_HANDLE should imply sm_ci != nullptr, but checking here anyway - if (sm_ci) { - sm_ci->SetCode(instrumented_spirv); - } - if (gpuav_settings.cache_instrumented_shaders && !cached) { - instrumented_shaders_cache_.Add(unique_shader_id, instrumented_spirv); - } - } - - chassis_state.shader_unique_id_maps[pipeline][stage] = unique_shader_id; + // + // This also could be because safe_VkShaderModuleCreateInfo is passed in the pNext of VkPipelineShaderStageCreateInfo + for (const auto &stage_state : pipe->stage_states) { + auto module_state = std::const_pointer_cast(stage_state.module_state); + ASSERT_AND_CONTINUE(module_state); + if (module_state->Handle()) continue; + + const VkShaderStageFlagBits stage = stage_state.GetStage(); + // Now find the corresponding VkShaderModuleCreateInfo + auto &stage_ci = + GetShaderStageCI(new_pipeline_ci, stage); + // We're modifying the copied, safe create info, which is ok to be non-const + auto sm_ci = + const_cast(reinterpret_cast( + vku::FindStructInPNextChain(stage_ci.pNext))); + + if (sm_ci) { + // Skip if we are in selective mode + if (gpuav_settings.select_instrumented_shaders && !CheckForGpuAvEnabled(sm_ci->pNext)) continue; + } else { + // If missing VkShaderModuleCreateInfo, for sure in GPL and not only want to check if there would be a shader + // anyway + if (pipe->HasFullState() || (!pipe->pre_raster_state && !pipe->fragment_shader_state)) break; + } + + // If the shader module's handle is non-null, then it was defined with CreateShaderModule and covered by the + // case above. Otherwise, it is being defined during CGPL time + if (chassis_state.shader_unique_id_maps.size() <= pipeline) { + chassis_state.shader_unique_id_maps.resize(pipeline + 1); + } + + uint32_t unique_shader_id = 0; + bool cached = false; + bool pass = false; + std::vector instrumented_spirv; + if (gpuav_settings.cache_instrumented_shaders) { + unique_shader_id = + hash_util::ShaderHash(module_state->spirv->words_.data(), module_state->spirv->words_.size()); + if (const auto spirv = instrumented_shaders_cache_.Get(unique_shader_id)) { + instrumented_spirv = *spirv; + cached = true; + } + } else { + unique_shader_id = unique_shader_module_id_++; + } + if (!cached) { + pass = InstrumentShader(module_state->spirv->words_, unique_shader_id, record_obj.location, instrumented_spirv); + } + if (cached || pass) { + module_state->gpu_validation_shader_id = unique_shader_id; + if (sm_ci) { + chassis_state.passed_in_shader_stage_ci = true; + sm_ci->SetCode(instrumented_spirv); + } else { + // TODO - GPL will hit here for executable pipelines, but we should be able to update the shader module + // handle since it can be found + } + if (gpuav_settings.cache_instrumented_shaders && !cached) { + instrumented_shaders_cache_.Add(unique_shader_id, instrumented_spirv); } } + + chassis_state.shader_unique_id_maps[pipeline][stage] = unique_shader_id; } } new_pipeline_create_infos->push_back(std::move(new_pipeline_ci)); @@ -754,7 +771,8 @@ void GpuShaderInstrumentor::PreCallRecordPipelineCreations(uint32_t count, const template void GpuShaderInstrumentor::PostCallRecordPipelineCreations(const uint32_t count, const CreateInfo *pCreateInfos, const VkAllocationCallbacks *pAllocator, VkPipeline *pPipelines, - const SafeCreateInfo &modified_create_infos) { + const SafeCreateInfo &modified_create_infos, + bool passed_in_shader_stage_ci) { for (uint32_t pipeline = 0; pipeline < count; ++pipeline) { auto pipeline_state = Get(pPipelines[pipeline]); if (!pipeline_state) continue; @@ -779,8 +797,12 @@ void GpuShaderInstrumentor::PostCallRecordPipelineCreations(const uint32_t count // the pipeline is used, so we have to keep another copy. if (module_state && module_state->spirv) code = module_state->spirv->words_; + VkShaderModule shader_module_handle = module_state->VkHandle(); + if (shader_module_handle == VK_NULL_HANDLE && passed_in_shader_stage_ci) { + shader_module_handle = kPipelineStageInfoHandle; + } shader_map_.insert_or_assign(module_state->gpu_validation_shader_id, pipeline_state->VkHandle(), - module_state->VkHandle(), VK_NULL_HANDLE, std::move(code)); + shader_module_handle, VK_NULL_HANDLE, std::move(code)); } } } diff --git a/layers/gpu/instrumentation/gpu_shader_instrumentor.h b/layers/gpu/instrumentation/gpu_shader_instrumentor.h index ca464b7fd8f..a534e9e727e 100644 --- a/layers/gpu/instrumentation/gpu_shader_instrumentor.h +++ b/layers/gpu/instrumentation/gpu_shader_instrumentor.h @@ -29,6 +29,11 @@ class Validator; } namespace gpu { + +// With VK_KHR_maintenance5 you can inline your VkShaderModuleCreateInfo via VkPipelineShaderStageCreateInfo::pNext +// We need a way to that a "null shader module" doesn't always mean "from shader object" +static const VkShaderModule kPipelineStageInfoHandle = CastFromUint64(0xEEEEEEEE); + class SpirvCache { public: void Add(uint32_t hash, std::vector spirv); @@ -151,7 +156,7 @@ class GpuShaderInstrumentor : public ValidationStateTracker { template void PostCallRecordPipelineCreations(const uint32_t count, const CreateInfo *pCreateInfos, const VkAllocationCallbacks *pAllocator, VkPipeline *pPipelines, - const SafeCreateInfo &modified_create_infos); + const SafeCreateInfo &modified_create_infos, bool passed_in_shader_stage_ci); // GPU-AV and DebugPrint are going to have a different way to do the actual shader instrumentation logic virtual bool InstrumentShader(const vvl::span &input, uint32_t unique_shader_id, const Location &loc, diff --git a/layers/state_tracker/pipeline_sub_state.cpp b/layers/state_tracker/pipeline_sub_state.cpp index cec6b0090f6..34a7b4c00ea 100644 --- a/layers/state_tracker/pipeline_sub_state.cpp +++ b/layers/state_tracker/pipeline_sub_state.cpp @@ -84,7 +84,7 @@ PreRasterState::PreRasterState(const vvl::Pipeline &p, const ValidationStateTrac if (const auto shader_ci = vku::FindStructInPNextChain(stage_ci.pNext)) { // don't need to worry about GroupDecoration in GPL spirv::StatelessData *stateless_data_stage = - (stateless_data && i < spirv::kCommonMaxShaderStages) ? &stateless_data[i] : nullptr; + (stateless_data && i < spirv::kCommonMaxGraphicsShaderStages) ? &stateless_data[i] : nullptr; auto spirv_module = std::make_shared(shader_ci->codeSize, shader_ci->pCode, stateless_data_stage); module_state = std::make_shared(VK_NULL_HANDLE, spirv_module, 0); if (stateless_data_stage) { @@ -198,7 +198,7 @@ void SetFragmentShaderInfoPrivate(FragmentShaderState &fs_state, const Validatio if (const auto shader_ci = vku::FindStructInPNextChain(create_info.pStages[i].pNext)) { // don't need to worry about GroupDecoration in GPL spirv::StatelessData *stateless_data_stage = - (stateless_data && i < spirv::kCommonMaxShaderStages) ? &stateless_data[i] : nullptr; + (stateless_data && i < spirv::kCommonMaxGraphicsShaderStages) ? &stateless_data[i] : nullptr; auto spirv_module = std::make_shared(shader_ci->codeSize, shader_ci->pCode, stateless_data_stage); module_state = std::make_shared(VK_NULL_HANDLE, spirv_module, 0); diff --git a/layers/state_tracker/shader_module.h b/layers/state_tracker/shader_module.h index b78c7cca34d..2107168c046 100644 --- a/layers/state_tracker/shader_module.h +++ b/layers/state_tracker/shader_module.h @@ -39,7 +39,7 @@ struct Module; static constexpr uint32_t kInvalidValue = std::numeric_limits::max(); // It is very rare to have more then 3 stages (really only geo/tess) and better to save memory/time for the 99% use cases -static const uint32_t kCommonMaxShaderStages = 3; +static const uint32_t kCommonMaxGraphicsShaderStages = 3; // This is the common info for both OpDecorate and OpMemberDecorate // Used to keep track of all decorations applied to any instruction diff --git a/tests/unit/debug_printf.cpp b/tests/unit/debug_printf.cpp index 8ccdb8201f8..02a43397f78 100644 --- a/tests/unit/debug_printf.cpp +++ b/tests/unit/debug_printf.cpp @@ -1656,8 +1656,7 @@ TEST_F(NegativeDebugPrintf, LocalSizeId) { m_errorMonitor->VerifyFound(); } -// TODO - https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8103 -TEST_F(NegativeDebugPrintf, DISABLED_Maintenance5) { +TEST_F(NegativeDebugPrintf, Maintenance5) { TEST_DESCRIPTION("Test SPIRV is still checked if using new pNext in VkPipelineShaderStageCreateInfo"); SetTargetApiVersion(VK_API_VERSION_1_3); AddRequiredExtensions(VK_KHR_MAINTENANCE_5_EXTENSION_NAME); diff --git a/tests/unit/gpu_av_buffer_device_address.cpp b/tests/unit/gpu_av_buffer_device_address.cpp index 86efc4e3bde..4c1179200ae 100644 --- a/tests/unit/gpu_av_buffer_device_address.cpp +++ b/tests/unit/gpu_av_buffer_device_address.cpp @@ -365,6 +365,100 @@ TEST_F(NegativeGpuAVBufferDeviceAddress, UVec3Array) { m_errorMonitor->VerifyFound(); } +TEST_F(NegativeGpuAVBufferDeviceAddress, Maintenance5) { + TEST_DESCRIPTION("Test SPIRV is still checked if using new pNext in VkPipelineShaderStageCreateInfo"); + SetTargetApiVersion(VK_API_VERSION_1_3); + AddRequiredExtensions(VK_KHR_MAINTENANCE_5_EXTENSION_NAME); + AddRequiredFeature(vkt::Feature::maintenance5); + RETURN_IF_SKIP(InitGpuVUBufferDeviceAddress()); + InitRenderTarget(); + + char const *fs_source = R"glsl( + #version 450 + #extension GL_EXT_buffer_reference : enable + + layout(buffer_reference, std430) readonly buffer IndexBuffer { + vec4 indices[]; + }; + + layout(set = 0, binding = 0) uniform foo { + IndexBuffer data; + int index; + } in_buffer; + + layout(location = 0) out vec4 color; + + void main() { + color = in_buffer.data.indices[in_buffer.index]; + } + )glsl"; + + std::vector vert_shader; + std::vector frag_shader; + this->GLSLtoSPV(&m_device->phy().limits_, VK_SHADER_STAGE_VERTEX_BIT, kVertexDrawPassthroughGlsl, vert_shader); + this->GLSLtoSPV(&m_device->phy().limits_, VK_SHADER_STAGE_FRAGMENT_BIT, fs_source, frag_shader); + + VkShaderModuleCreateInfo module_create_info_vert = vku::InitStructHelper(); + module_create_info_vert.pCode = vert_shader.data(); + module_create_info_vert.codeSize = vert_shader.size() * sizeof(uint32_t); + + VkPipelineShaderStageCreateInfo stage_ci_vert = vku::InitStructHelper(&module_create_info_vert); + stage_ci_vert.stage = VK_SHADER_STAGE_VERTEX_BIT; + stage_ci_vert.module = VK_NULL_HANDLE; + stage_ci_vert.pName = "main"; + + VkShaderModuleCreateInfo module_create_info_frag = vku::InitStructHelper(); + module_create_info_frag.pCode = frag_shader.data(); + module_create_info_frag.codeSize = frag_shader.size() * sizeof(uint32_t); + + VkPipelineShaderStageCreateInfo stage_ci_frag = vku::InitStructHelper(&module_create_info_frag); + stage_ci_frag.stage = VK_SHADER_STAGE_FRAGMENT_BIT; + stage_ci_frag.module = VK_NULL_HANDLE; + stage_ci_frag.pName = "main"; + + OneOffDescriptorSet descriptor_set(m_device, { + {0, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}, + }); + const vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set.layout_}); + + CreatePipelineHelper pipe(*this); + pipe.shader_stages_ = {stage_ci_vert, stage_ci_frag}; + pipe.gp_ci_.layout = pipeline_layout.handle(); + pipe.CreateGraphicsPipeline(); + + VkMemoryPropertyFlags mem_props = VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT; + VkMemoryAllocateFlagsInfo allocate_flag_info = vku::InitStructHelper(); + allocate_flag_info.flags = VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BIT; + vkt::Buffer block_buffer(*m_device, 16, VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT_KHR, mem_props, &allocate_flag_info); + + vkt::Buffer in_buffer(*m_device, 16, VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT, mem_props); + + VkDeviceAddress block_ptr = block_buffer.address(); + const uint32_t n_reads = 64; // way too large + + uint8_t *in_buffer_ptr = (uint8_t *)in_buffer.memory().map(); + memcpy(in_buffer_ptr, &block_ptr, sizeof(VkDeviceAddress)); + memcpy(in_buffer_ptr + sizeof(VkDeviceAddress), &n_reads, sizeof(uint32_t)); + in_buffer.memory().unmap(); + + descriptor_set.WriteDescriptorBufferInfo(0, in_buffer.handle(), 0, VK_WHOLE_SIZE); + descriptor_set.UpdateDescriptorSets(); + + m_commandBuffer->begin(); + m_commandBuffer->BeginRenderPass(m_renderPassBeginInfo); + vk::CmdBindPipeline(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_GRAPHICS, pipe.Handle()); + vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline_layout.handle(), 0, 1, + &descriptor_set.set_, 0, nullptr); + vk::CmdDraw(m_commandBuffer->handle(), 3, 1, 0, 0); + vk::CmdEndRenderPass(m_commandBuffer->handle()); + m_commandBuffer->end(); + + m_errorMonitor->SetDesiredError("UNASSIGNED-Device address out of bounds", 6); + m_default_queue->Submit(*m_commandBuffer); + m_default_queue->Wait(); + m_errorMonitor->VerifyFound(); +} + // https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7462 TEST_F(NegativeGpuAVBufferDeviceAddress, DISABLED_ArrayOfStruct) { SetTargetApiVersion(VK_API_VERSION_1_2);