From f0f4f9c5665924026797bb5a661b470f0fbaa66a Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Wed, 21 May 2025 17:40:09 +0300 Subject: [PATCH] brw: fix vertex attribute offset computation The formula uses scalar indices (4bytes), not slots (16bytes). We also incorrectly passed a scalar (vertex case) & slot (mesh case) offset in the push constants. Use slots instead so that the value is smaller and we can pack more stuff into fs_msaa_flags. Signed-off-by: Lionel Landwerlin Fixes: 18bbcf9a63 ("intel: introduce new VUE layout for separate compiled shader with mesh") Reviewed-by: Ivan Briano Part-of: --- src/intel/compiler/brw_compile_fs.cpp | 17 ++++++++++++----- src/intel/compiler/brw_nir.c | 19 ++++++++++++------- src/intel/compiler/intel_shader_enums.h | 2 ++ 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/intel/compiler/brw_compile_fs.cpp b/src/intel/compiler/brw_compile_fs.cpp index 3099ea6be66..4c7fc205e84 100644 --- a/src/intel/compiler/brw_compile_fs.cpp +++ b/src/intel/compiler/brw_compile_fs.cpp @@ -1944,10 +1944,16 @@ brw_compute_sbe_per_vertex_urb_read(const struct intel_vue_map *prev_stage_vue_m * use that. */ if (wm_prog_data->urb_setup[VARYING_SLOT_PRIMITIVE_ID] >= 0) { - if (first_slot == INT32_MAX) - first_slot = 0; - primitive_id_slot = - first_slot + wm_prog_data->urb_setup[VARYING_SLOT_PRIMITIVE_ID]; + if (first_slot == INT32_MAX) { + first_slot = + wm_prog_data->urb_setup[VARYING_SLOT_PRIMITIVE_ID]; + } + /* urb_setup[VARYING_SLOT_PRIMITIVE_ID] is relative to the + * first read slot, so bring primitive_id_slot back into the + * absolute indexing of the VUE. + */ + primitive_id_slot = first_slot + + wm_prog_data->urb_setup[VARYING_SLOT_PRIMITIVE_ID]; } else { primitive_id_slot = ++last_slot; } @@ -1955,9 +1961,10 @@ brw_compute_sbe_per_vertex_urb_read(const struct intel_vue_map *prev_stage_vue_m primitive_id_slot = prev_stage_vue_map->varying_to_slot[VARYING_SLOT_PRIMITIVE_ID]; } + first_slot = MIN2(primitive_id_slot, first_slot); last_slot = MAX2(primitive_id_slot, last_slot); - *out_primitive_id_offset = 4 * (primitive_id_slot - first_slot); + *out_primitive_id_offset = primitive_id_slot - first_slot; } } diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c index a6f39a4bf2e..a3e78aef381 100644 --- a/src/intel/compiler/brw_nir.c +++ b/src/intel/compiler/brw_nir.c @@ -706,9 +706,11 @@ brw_nir_lower_fs_inputs(nir_shader *nir, if (key->base.vue_layout == INTEL_VUE_LAYOUT_SEPARATE_MESH && (nir->info.inputs_read & VARYING_BIT_PRIMITIVE_ID)) { nir_builder _b = nir_builder_at(nir_before_impl(nir_shader_get_entrypoint(nir))), *b = &_b; - nir_def *index = nir_ushr_imm(b, - nir_load_fs_msaa_intel(b), - INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_OFFSET); + nir_def *index = nir_ubitfield_extract_imm( + b, + nir_load_fs_msaa_intel(b), + INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_OFFSET, + INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_SIZE); nir_def *max_poly = nir_load_max_polygon_intel(b); /* Build the per-vertex offset into the attribute section of the thread * payload. There is always one GRF of padding in front. @@ -723,18 +725,21 @@ brw_nir_lower_fs_inputs(nir_shader *nir, * Then an additional offset needs to added to handle how multiple * polygon data is interleaved. */ + nir_def *scalar_index = nir_imul_imm(b, index, 4); nir_def *per_vertex_offset = nir_iadd_imm( b, devinfo->ver >= 20 ? nir_iadd(b, - nir_imul(b, nir_udiv_imm(b, index, 5), nir_imul_imm(b, max_poly, 64)), - nir_imul_imm(b, nir_umod_imm(b, index, 5), 12)) : + nir_imul(b, nir_udiv_imm(b, scalar_index, 5), + nir_imul_imm(b, max_poly, 64)), + nir_imul_imm(b, nir_umod_imm(b, scalar_index, 5), 12)) : nir_iadd_imm( b, nir_iadd( b, - nir_imul(b, nir_udiv_imm(b, index, 2), nir_imul_imm(b, max_poly, 32)), - nir_imul_imm(b, nir_umod_imm(b, index, 2), 16)), + nir_imul(b, nir_udiv_imm(b, scalar_index, 2), + nir_imul_imm(b, max_poly, 32)), + nir_imul_imm(b, nir_umod_imm(b, scalar_index, 2), 16)), 12), devinfo->grf_size); /* When the attribute index is INTEL_MSAA_FLAG_PRIMITIVE_ID_MESH_INDEX, diff --git a/src/intel/compiler/intel_shader_enums.h b/src/intel/compiler/intel_shader_enums.h index 55d579f3e3f..743b70dcaf9 100644 --- a/src/intel/compiler/intel_shader_enums.h +++ b/src/intel/compiler/intel_shader_enums.h @@ -31,6 +31,7 @@ intel_sometimes_invert(enum intel_sometimes x) } #define INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_OFFSET (20) +#define INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_SIZE (6) #define INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_MESH (32) enum intel_msaa_flags { @@ -472,6 +473,7 @@ intel_fs_msaa_flags(struct intel_fs_params params) if (params.alpha_to_coverage) fs_msaa_flags |= INTEL_MSAA_FLAG_ALPHA_TO_COVERAGE; + assert(params.primitive_id_index < (1u << INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_SIZE)); fs_msaa_flags |= (enum intel_msaa_flags)( params.primitive_id_index << INTEL_MSAA_FLAG_PRIMITIVE_ID_INDEX_OFFSET);