anv: remove descriptor array bounds checking
authorLionel Landwerlin <lionel.g.landwerlin@intel.com>
Mon, 3 Jul 2023 14:16:28 +0000 (17:16 +0300)
committerLionel Landwerlin <lionel.g.landwerlin@intel.com>
Wed, 9 Aug 2023 06:00:12 +0000 (09:00 +0300)
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 <lionel.g.landwerlin@intel.com>
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17545>

src/intel/vulkan/anv_nir_apply_pipeline_layout.c

index 29a2c1a..58862b3 100644 (file)
@@ -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,