From 65acc81e9d2435c44bbdc6f4fb1ea7bcc453463a Mon Sep 17 00:00:00 2001 From: Samuel Pitoiset Date: Tue, 16 Jul 2024 13:40:54 +0200 Subject: [PATCH] radv: fix shaders cache corruption with indirect pipeline binds Indirect pipeline binds force indirect descriptor sets and this needs to be in the shader stage key, otherwise two shaders might result in the same pipeline cache key. Fixes: b1ba02e707b ("radv: force using indirect descriptor sets for indirect compute pipelines") Signed-off-by: Samuel Pitoiset Part-of: --- src/amd/vulkan/radv_pipeline.c | 3 +++ src/amd/vulkan/radv_pipeline_compute.c | 10 ++++------ src/amd/vulkan/radv_pipeline_compute.h | 3 +-- src/amd/vulkan/radv_pipeline_graphics.c | 4 ++-- src/amd/vulkan/radv_pipeline_rt.c | 2 +- src/amd/vulkan/radv_shader.h | 3 +++ src/amd/vulkan/radv_shader_info.c | 4 ++-- src/amd/vulkan/radv_shader_info.h | 2 +- src/amd/vulkan/radv_shader_object.c | 2 +- 9 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c index 59381f1689c..77cbc2e7f77 100644 --- a/src/amd/vulkan/radv_pipeline.c +++ b/src/amd/vulkan/radv_pipeline.c @@ -148,6 +148,9 @@ radv_pipeline_get_shader_key(const struct radv_device *device, const VkPipelineS if (flags & VK_PIPELINE_CREATE_2_DISABLE_OPTIMIZATION_BIT_KHR) key.optimisations_disabled = 1; + if (flags & VK_PIPELINE_CREATE_INDIRECT_BINDABLE_BIT_NV) + key.indirect_bindable = 1; + if (stage->stage & RADV_GRAPHICS_STAGE_BITS) { key.version = instance->drirc.override_graphics_shader_version; } else if (stage->stage & RADV_RT_STAGE_BITS) { diff --git a/src/amd/vulkan/radv_pipeline_compute.c b/src/amd/vulkan/radv_pipeline_compute.c index 772ae574c8e..fafd4972762 100644 --- a/src/amd/vulkan/radv_pipeline_compute.c +++ b/src/amd/vulkan/radv_pipeline_compute.c @@ -97,7 +97,7 @@ radv_compute_pipeline_init(struct radv_compute_pipeline *pipeline, const struct struct radv_shader * radv_compile_cs(struct radv_device *device, struct vk_pipeline_cache *cache, struct radv_shader_stage *cs_stage, - bool keep_executable_info, bool keep_statistic_info, bool is_internal, bool is_indirect_bindable, + bool keep_executable_info, bool keep_statistic_info, bool is_internal, struct radv_shader_binary **cs_binary) { struct radv_shader *cs_shader; @@ -113,7 +113,7 @@ radv_compile_cs(struct radv_device *device, struct vk_pipeline_cache *cache, str /* Run the shader info pass. */ radv_nir_shader_info_init(cs_stage->stage, MESA_SHADER_NONE, &cs_stage->info); radv_nir_shader_info_pass(device, cs_stage->nir, &cs_stage->layout, &cs_stage->key, NULL, RADV_PIPELINE_COMPUTE, - false, is_indirect_bindable, &cs_stage->info); + false, &cs_stage->info); radv_declare_shader_args(device, NULL, &cs_stage->info, MESA_SHADER_COMPUTE, MESA_SHADER_NONE, &cs_stage->args); @@ -209,13 +209,11 @@ radv_compute_pipeline_compile(const VkComputePipelineCreateInfo *pCreateInfo, st const struct radv_shader_stage_key stage_key = radv_pipeline_get_shader_key(device, &pCreateInfo->stage, pipeline->base.create_flags, pCreateInfo->pNext); - const bool is_indirect_bindable = !!(pipeline->base.create_flags & VK_PIPELINE_CREATE_INDIRECT_BINDABLE_BIT_NV); radv_pipeline_stage_init(pStage, pipeline_layout, &stage_key, &cs_stage); - pipeline->base.shaders[MESA_SHADER_COMPUTE] = - radv_compile_cs(device, cache, &cs_stage, keep_executable_info, keep_statistic_info, pipeline->base.is_internal, - is_indirect_bindable, &cs_binary); + pipeline->base.shaders[MESA_SHADER_COMPUTE] = radv_compile_cs( + device, cache, &cs_stage, keep_executable_info, keep_statistic_info, pipeline->base.is_internal, &cs_binary); cs_stage.feedback.duration += os_time_get_nano() - stage_start; diff --git a/src/amd/vulkan/radv_pipeline_compute.h b/src/amd/vulkan/radv_pipeline_compute.h index 9c4b22b600b..559a6d5d8fc 100644 --- a/src/amd/vulkan/radv_pipeline_compute.h +++ b/src/amd/vulkan/radv_pipeline_compute.h @@ -54,8 +54,7 @@ void radv_compute_pipeline_init(struct radv_compute_pipeline *pipeline, const st struct radv_shader *radv_compile_cs(struct radv_device *device, struct vk_pipeline_cache *cache, struct radv_shader_stage *cs_stage, bool keep_executable_info, - bool keep_statistic_info, bool is_internal, bool is_indirect_bindable, - struct radv_shader_binary **cs_binary); + bool keep_statistic_info, bool is_internal, struct radv_shader_binary **cs_binary); VkResult radv_compute_pipeline_create(VkDevice _device, VkPipelineCache _cache, const VkComputePipelineCreateInfo *pCreateInfo, diff --git a/src/amd/vulkan/radv_pipeline_graphics.c b/src/amd/vulkan/radv_pipeline_graphics.c index 85e8edc45ff..57a130e536f 100644 --- a/src/amd/vulkan/radv_pipeline_graphics.c +++ b/src/amd/vulkan/radv_pipeline_graphics.c @@ -2001,7 +2001,7 @@ radv_fill_shader_info(struct radv_device *device, const enum radv_pipeline_type } radv_nir_shader_info_pass(device, stages[i].nir, &stages[i].layout, &stages[i].key, gfx_state, pipeline_type, - consider_force_vrs, false, &stages[i].info); + consider_force_vrs, &stages[i].info); } radv_nir_shader_info_link(device, gfx_state, stages); @@ -2082,7 +2082,7 @@ radv_create_gs_copy_shader(struct radv_device *device, struct vk_pipeline_cache }; radv_nir_shader_info_init(gs_copy_stage.stage, MESA_SHADER_FRAGMENT, &gs_copy_stage.info); radv_nir_shader_info_pass(device, nir, &gs_stage->layout, &gs_stage->key, gfx_state, RADV_PIPELINE_GRAPHICS, false, - false, &gs_copy_stage.info); + &gs_copy_stage.info); gs_copy_stage.info.wave_size = 64; /* Wave32 not supported. */ gs_copy_stage.info.workgroup_size = 64; /* HW VS: separate waves, no workgroups */ gs_copy_stage.info.so = gs_info->so; diff --git a/src/amd/vulkan/radv_pipeline_rt.c b/src/amd/vulkan/radv_pipeline_rt.c index 14b2e14ff4d..2a69006d70d 100644 --- a/src/amd/vulkan/radv_pipeline_rt.c +++ b/src/amd/vulkan/radv_pipeline_rt.c @@ -361,7 +361,7 @@ radv_rt_nir_to_asm(struct radv_device *device, struct vk_pipeline_cache *cache, nir_shader_gather_info(stage->nir, nir_shader_get_entrypoint(stage->nir)); radv_nir_shader_info_init(stage->stage, MESA_SHADER_NONE, &stage->info); radv_nir_shader_info_pass(device, stage->nir, &stage->layout, &stage->key, NULL, RADV_PIPELINE_RAY_TRACING, false, - false, &stage->info); + &stage->info); /* Declare shader arguments. */ radv_declare_shader_args(device, NULL, &stage->info, stage->stage, MESA_SHADER_NONE, &stage->args); diff --git a/src/amd/vulkan/radv_shader.h b/src/amd/vulkan/radv_shader.h index 954e718676a..a3f152aebac 100644 --- a/src/amd/vulkan/radv_shader.h +++ b/src/amd/vulkan/radv_shader.h @@ -84,6 +84,9 @@ struct radv_shader_stage_key { /* Whether the mesh shader is used with a task shader. */ uint8_t has_task_shader : 1; + + /* Whether the shader is used with indirect pipeline binds. */ + uint8_t indirect_bindable : 1; }; struct radv_ps_epilog_key { diff --git a/src/amd/vulkan/radv_shader_info.c b/src/amd/vulkan/radv_shader_info.c index 3e85d018dd5..776bc99ac77 100644 --- a/src/amd/vulkan/radv_shader_info.c +++ b/src/amd/vulkan/radv_shader_info.c @@ -1121,7 +1121,7 @@ void radv_nir_shader_info_pass(struct radv_device *device, const struct nir_shader *nir, const struct radv_shader_layout *layout, const struct radv_shader_stage_key *stage_key, const struct radv_graphics_state_key *gfx_state, const enum radv_pipeline_type pipeline_type, - bool consider_force_vrs, bool is_indirect_bindable, struct radv_shader_info *info) + bool consider_force_vrs, struct radv_shader_info *info) { const struct radv_physical_device *pdev = radv_device_physical(device); struct nir_function *func = (struct nir_function *)exec_list_get_head_const(&nir->functions); @@ -1232,7 +1232,7 @@ radv_nir_shader_info_pass(struct radv_device *device, const struct nir_shader *n info->user_data_0 = radv_get_user_data_0(device, info); info->merged_shader_compiled_separately = radv_is_merged_shader_compiled_separately(device, info); - info->force_indirect_desc_sets = info->merged_shader_compiled_separately || is_indirect_bindable; + info->force_indirect_desc_sets = info->merged_shader_compiled_separately || stage_key->indirect_bindable; switch (nir->info.stage) { case MESA_SHADER_COMPUTE: diff --git a/src/amd/vulkan/radv_shader_info.h b/src/amd/vulkan/radv_shader_info.h index 781c0717caf..e072c9ffd5b 100644 --- a/src/amd/vulkan/radv_shader_info.h +++ b/src/amd/vulkan/radv_shader_info.h @@ -317,7 +317,7 @@ void radv_nir_shader_info_pass(struct radv_device *device, const struct nir_shad const struct radv_shader_layout *layout, const struct radv_shader_stage_key *stage_key, const struct radv_graphics_state_key *gfx_state, const enum radv_pipeline_type pipeline_type, bool consider_force_vrs, - bool is_indirect_bindable, struct radv_shader_info *info); + struct radv_shader_info *info); void gfx10_get_ngg_info(const struct radv_device *device, struct radv_shader_info *es_info, struct radv_shader_info *gs_info, struct gfx10_ngg_info *out); diff --git a/src/amd/vulkan/radv_shader_object.c b/src/amd/vulkan/radv_shader_object.c index db6004719ce..e36196a6dc9 100644 --- a/src/amd/vulkan/radv_shader_object.c +++ b/src/amd/vulkan/radv_shader_object.c @@ -248,7 +248,7 @@ radv_shader_object_init_compute(struct radv_shader_object *shader_obj, struct ra radv_shader_stage_init(pCreateInfo, &stage); - struct radv_shader *cs_shader = radv_compile_cs(device, NULL, &stage, true, false, false, false, &cs_binary); + struct radv_shader *cs_shader = radv_compile_cs(device, NULL, &stage, true, false, false, &cs_binary); ralloc_free(stage.nir);