anv: fixup push descriptor shader analysis

There are a couple mistakes here :

   - using a bitfield as an index to generate a bitfield...

   - in anv_nir_push_desc_ubo_fully_promoted(), confusing binding
     table access of the descriptor buffer with actual descriptors

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: ff91c5ca42 ("anv: add analysis for push descriptor uses and store it in shader cache")
Reviewed-by: Ivan Briano <ivan.briano@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/27504>
This commit is contained in:
Lionel Landwerlin 2024-02-07 10:16:57 +02:00 committed by Marge Bot
parent 735fe243a7
commit cf193af762
3 changed files with 80 additions and 38 deletions

View file

@ -125,8 +125,7 @@ brw_nir_ubo_surface_index_is_pushable(nir_src src)
if (intrin && intrin->intrinsic == nir_intrinsic_resource_intel) {
return (nir_intrinsic_resource_access_intel(intrin) &
nir_resource_intel_pushable) &&
nir_src_is_const(intrin->src[1]);
nir_resource_intel_pushable);
}
return nir_src_is_const(src);
@ -149,6 +148,14 @@ brw_nir_ubo_surface_index_get_push_block(nir_src src)
return nir_intrinsic_resource_block_intel(intrin);
}
/* This helper return the binding table index of a surface access (any
* buffer/image/etc...). It works off the source of one of the intrinsics
* (load_ubo, load_ssbo, store_ssbo, load_image, store_image, etc...).
*
* If the source is constant, then this is the binding table index. If we're
* going through a resource_intel intel intrinsic, then we need to check
* src[1] of that intrinsic.
*/
static inline unsigned
brw_nir_ubo_surface_index_get_bti(nir_src src)
{
@ -158,8 +165,19 @@ brw_nir_ubo_surface_index_get_bti(nir_src src)
assert(src.ssa->parent_instr->type == nir_instr_type_intrinsic);
nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(src.ssa->parent_instr);
assert(intrin->intrinsic == nir_intrinsic_resource_intel);
assert(nir_src_is_const(intrin->src[1]));
if (!intrin || intrin->intrinsic != nir_intrinsic_resource_intel)
return UINT32_MAX;
/* In practice we could even drop this intrinsic because the bindless
* access always operate from a base offset coming from a push constant, so
* they can never be constant.
*/
if (nir_intrinsic_resource_access_intel(intrin) &
nir_resource_intel_bindless)
return UINT32_MAX;
if (!nir_src_is_const(intrin->src[1]))
return UINT32_MAX;
return nir_src_as_uint(intrin->src[1]);
}

View file

@ -126,18 +126,17 @@ anv_nir_loads_push_desc_buffer(nir_shader *nir,
if (intrin->intrinsic != nir_intrinsic_load_ubo)
continue;
const nir_const_value *const_bt_idx =
nir_src_as_const_value(intrin->src[0]);
if (const_bt_idx == NULL)
const unsigned bt_idx =
brw_nir_ubo_surface_index_get_bti(intrin->src[0]);
if (bt_idx == UINT32_MAX)
continue;
const unsigned bt_idx = const_bt_idx[0].u32;
const struct anv_pipeline_binding *binding =
&bind_map->surface_to_descriptor[bt_idx];
if (binding->set == ANV_DESCRIPTOR_SET_DESCRIPTORS &&
binding->index == push_set)
binding->index == push_set) {
return true;
}
}
}
}
@ -162,6 +161,7 @@ anv_nir_push_desc_ubo_fully_promoted(nir_shader *nir,
if (push_set_layout == NULL)
return 0;
/* Assume every UBO can be promoted first. */
uint32_t ubos_fully_promoted = 0;
for (uint32_t b = 0; b < push_set_layout->binding_count; b++) {
const struct anv_descriptor_set_binding_layout *bind_layout =
@ -174,6 +174,10 @@ anv_nir_push_desc_ubo_fully_promoted(nir_shader *nir,
ubos_fully_promoted |= BITFIELD_BIT(bind_layout->descriptor_index);
}
/* For each load_ubo intrinsic, if the descriptor index or the offset is
* not a constant, we could not promote to push constant. Then check the
* offset + size against the push ranges.
*/
nir_foreach_function_impl(impl, nir) {
nir_foreach_block(block, impl) {
nir_foreach_instr(instr, block) {
@ -184,45 +188,65 @@ anv_nir_push_desc_ubo_fully_promoted(nir_shader *nir,
if (intrin->intrinsic != nir_intrinsic_load_ubo)
continue;
if (!brw_nir_ubo_surface_index_is_pushable(intrin->src[0]))
/* Don't check the load_ubo from descriptor buffers */
nir_intrinsic_instr *resource =
intrin->src[0].ssa->parent_instr->type == nir_instr_type_intrinsic ?
nir_instr_as_intrinsic(intrin->src[0].ssa->parent_instr) : NULL;
if (resource == NULL || resource->intrinsic != nir_intrinsic_resource_intel)
continue;
const unsigned bt_idx =
brw_nir_ubo_surface_index_get_bti(intrin->src[0]);
/* Skip if this isn't a load from push descriptor buffer. */
const struct anv_pipeline_binding *binding =
&bind_map->surface_to_descriptor[bt_idx];
if (binding->set != push_set)
/* Skip load_ubo not loading from the push descriptor */
if (nir_intrinsic_desc_set(resource) != push_set)
continue;
uint32_t binding = nir_intrinsic_binding(resource);
/* If we have indirect indexing in the binding, no push promotion
* in possible for the entire binding.
*/
if (!nir_src_is_const(resource->src[1])) {
for (uint32_t i = 0; i < push_set_layout->binding[binding].array_size; i++) {
ubos_fully_promoted &=
~BITFIELD_BIT(push_set_layout->binding[binding].descriptor_index + i);
}
continue;
}
const nir_const_value *const_bt_id =
nir_src_as_const_value(resource->src[1]);
uint32_t bt_id = const_bt_id[0].u32;
const struct anv_pipeline_binding *pipe_bind =
&bind_map->surface_to_descriptor[bt_id];
const uint32_t desc_idx =
push_set_layout->binding[binding->binding].descriptor_index;
assert(desc_idx < MAX_PUSH_DESCRIPTORS);
bool promoted = false;
push_set_layout->binding[binding].descriptor_index;
/* If the offset in the entry is dynamic, we can't tell if
* promoted or not.
*/
const nir_const_value *const_load_offset =
nir_src_as_const_value(intrin->src[1]);
if (const_load_offset != NULL) {
/* Check if the load was promoted to a push constant. */
const unsigned load_offset = const_load_offset[0].u32;
const int load_bytes = nir_intrinsic_dest_components(intrin) *
(intrin->def.bit_size / 8);
if (const_load_offset == NULL) {
ubos_fully_promoted &= ~BITFIELD_BIT(desc_idx);
continue;
}
for (unsigned i = 0; i < ARRAY_SIZE(bind_map->push_ranges); i++) {
if (bind_map->push_ranges[i].set == binding->set &&
bind_map->push_ranges[i].index == desc_idx &&
bind_map->push_ranges[i].start * 32 <= load_offset &&
(bind_map->push_ranges[i].start +
bind_map->push_ranges[i].length) * 32 >=
(load_offset + load_bytes)) {
promoted = true;
break;
}
/* Check if the load was promoted to a push constant. */
const unsigned load_offset = const_load_offset[0].u32;
const int load_bytes = nir_intrinsic_dest_components(intrin) *
(intrin->def.bit_size / 8);
bool promoted = false;
for (unsigned i = 0; i < ARRAY_SIZE(bind_map->push_ranges); i++) {
if (bind_map->push_ranges[i].set == pipe_bind->set &&
bind_map->push_ranges[i].index == desc_idx &&
bind_map->push_ranges[i].start * 32 <= load_offset &&
(bind_map->push_ranges[i].start +
bind_map->push_ranges[i].length) * 32 >=
(load_offset + load_bytes)) {
promoted = true;
break;
}
}

View file

@ -1734,7 +1734,7 @@ anv_pipeline_account_shader(struct anv_pipeline *pipeline,
if (shader->push_desc_info.used_set_buffer) {
pipeline->use_push_descriptor_buffer |=
BITFIELD_BIT(mesa_to_vk_shader_stage(shader->stage));
mesa_to_vk_shader_stage(shader->stage);
}
if (shader->push_desc_info.used_descriptors &
~shader->push_desc_info.fully_promoted_ubo_descriptors)