From 059e82a4699c21490a79f5dfb6a4976be3856d32 Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Mon, 3 Jul 2023 17:16:28 +0300 Subject: [PATCH] anv: remove descriptor array bounds checking We cannot find anything in the Vulkan spec requiring this. D3D12 [1] says it's undefined as long as it doesn't crash the OS : "Out of bounds indexing of any descriptor table from the shader results in a largely undefined memory access, including the possibility of reading arbitrary in-process memory as if it is a hardware state descriptor and living with the consequence of what the hardware does with that. This could produce a device reset, but will not crash Windows." [1] : https://learn.microsoft.com/en-us/windows/win32/direct3d12/advanced-use-of-descriptor-tables#out-of-bounds-indexing Found 2 titles affected by this change Some pretty good results on Cyberpunk 2077 : Totals from 10285 (100.00% of 10285) affected shaders: Instrs: 7638709 -> 7517360 (-1.59%); split: -1.64%, +0.05% Cycles: 148047414 -> 148470916 (+0.29%); split: -0.83%, +1.12% Subgroup size: 112544 -> 112576 (+0.03%); split: +0.04%, -0.01% Spill count: 98 -> 90 (-8.16%) Fill count: 90 -> 82 (-8.89%) Max live registers: 495274 -> 479502 (-3.18%); split: -3.21%, +0.03% Max dispatch width: 87824 -> 91168 (+3.81%); split: +4.10%, -0.29% Gaining 297 shaders in SIMD16/32, loosing 16 SIMD32 shaders Some not so good results on Strange Brigade : Totals from 4027 (100.00% of 4027) affected shaders: Instrs: 2080355 -> 2013880 (-3.20%); split: -3.20%, +0.01% Cycles: 25405149 -> 25170579 (-0.92%); split: -1.37%, +0.45% Max live registers: 167303 -> 168958 (+0.99%) Max dispatch width: 33264 -> 32496 (-2.31%) Loosing 96 SIMD16 shaders. Signed-off-by: Lionel Landwerlin Reviewed-by: Ivan Briano Part-of: --- src/intel/vulkan/anv_nir_apply_pipeline_layout.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c index 29a2c1a..58862b3 100644 --- a/src/intel/vulkan/anv_nir_apply_pipeline_layout.c +++ b/src/intel/vulkan/anv_nir_apply_pipeline_layout.c @@ -48,7 +48,6 @@ struct apply_pipeline_layout_state { const struct anv_physical_device *pdevice; const struct anv_pipeline_sets_layout *layout; - bool add_bounds_checks; nir_address_format desc_addr_format; nir_address_format ssbo_addr_format; nir_address_format ubo_addr_format; @@ -683,11 +682,6 @@ build_surface_index_for_binding(nir_builder *b, const bool is_bindless = is_binding_bindless(set, binding, false /* sampler */, state); - if (state->add_bounds_checks) { - array_index = nir_umin(b, array_index, - nir_imm_int(b, bind_layout->array_size - 1)); - } - nir_ssa_def *set_offset, *surface_index; if (is_bindless) { if (state->layout->type == ANV_PIPELINE_DESCRIPTOR_SET_LAYOUT_TYPE_INDIRECT) { @@ -830,10 +824,6 @@ build_buffer_dynamic_offset_for_res_index(nir_builder *b, struct apply_pipeline_layout_state *state) { nir_ssa_def *dyn_offset_idx = nir_iadd(b, dyn_offset_base, array_index); - if (state->add_bounds_checks) { - dyn_offset_idx = nir_umin(b, dyn_offset_idx, - nir_imm_int(b, MAX_DYNAMIC_BUFFERS - 1)); - } nir_ssa_def *dyn_load = nir_load_push_constant(b, 1, 32, nir_imul_imm(b, dyn_offset_idx, 4), @@ -882,10 +872,6 @@ build_indirect_buffer_addr_for_res_index(nir_builder *b, */ nir_ssa_def *dyn_offset_idx = nir_iadd(b, res.dyn_offset_base, res.array_index); - if (state->add_bounds_checks) { - dyn_offset_idx = nir_umin(b, dyn_offset_idx, - nir_imm_int(b, MAX_DYNAMIC_BUFFERS)); - } nir_ssa_def *dyn_load = nir_load_push_constant(b, 1, 32, nir_imul_imm(b, dyn_offset_idx, 4), @@ -1571,18 +1557,13 @@ lower_tex_deref(nir_builder *b, nir_tex_instr *tex, const bool is_sampler = deref_src_type == nir_tex_src_sampler_deref; const unsigned set = var->data.descriptor_set; const unsigned binding = var->data.binding; - const struct anv_descriptor_set_binding_layout *bind_layout = - &state->layout->set[set].layout->binding[binding]; const bool bindless = is_binding_bindless(set, binding, is_sampler, state); - unsigned array_size = bind_layout->array_size; nir_ssa_def *array_index = NULL; if (deref->deref_type != nir_deref_type_var) { assert(deref->deref_type == nir_deref_type_array); array_index = nir_ssa_for_src(b, deref->arr.index, 1); - if (state->add_bounds_checks) - array_index = nir_umin(b, array_index, nir_imm_int(b, array_size - 1)); } else { array_index = nir_imm_int(b, 0); } @@ -1883,7 +1864,6 @@ anv_nir_apply_pipeline_layout(nir_shader *shader, struct apply_pipeline_layout_state state = { .pdevice = pdevice, .layout = layout, - .add_bounds_checks = robust_buffer_access, .desc_addr_format = bindless_stage ? nir_address_format_64bit_global_32bit_offset : nir_address_format_32bit_index_offset, -- 2.7.4