From 35cdddf63276d000e0dae364e8117b6daad4fbde Mon Sep 17 00:00:00 2001 From: Faith Ekstrand Date: Thu, 6 Nov 2025 21:42:00 -0500 Subject: [PATCH] nir: Simplify assign_io_var_locations() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The size and stage parameters are left-overs from history. Originally, the function acted on a list and so it needed an explicit stage and size output. Now that it takes a NIR shader and a mode, we can just take the stage from the shader and set num_(in|out)puts. The one caller that actually used the explicit output parameter was turnip. However, given that the helper sorts and re-numbers all the I/O variables, it's not like changing num_(in|out)puts instead of writing it to some other location is that big of a deal. Reviewed-by: Samuel Pitoiset Reviewed-by: Boris Brezillon Reviewed-by: Alejandro Piñeiro Part-of: --- src/amd/vulkan/nir/radv_nir_lower_io.c | 2 +- src/broadcom/vulkan/v3dv_pipeline.c | 21 ++++++------------- src/compiler/nir/nir.h | 5 +---- src/compiler/nir/nir_linking_helpers.c | 17 +++++++++------ src/freedreno/ir3/ir3_shader.c | 9 ++------ src/freedreno/vulkan/tu_clear_blit.cc | 4 ++-- .../vulkan/tu_nir_lower_multiview.cc | 5 ++--- src/freedreno/vulkan/tu_shader.cc | 4 ++-- src/gallium/frontends/lavapipe/lvp_pipeline.c | 5 ++--- src/panfrost/vulkan/panvk_vX_shader.c | 9 +++----- 10 files changed, 32 insertions(+), 49 deletions(-) diff --git a/src/amd/vulkan/nir/radv_nir_lower_io.c b/src/amd/vulkan/nir/radv_nir_lower_io.c index 516f46fc949..425ffdda65f 100644 --- a/src/amd/vulkan/nir/radv_nir_lower_io.c +++ b/src/amd/vulkan/nir/radv_nir_lower_io.c @@ -169,7 +169,7 @@ radv_nir_lower_io(struct radv_device *device, nir_shader *nir) /* The total number of shader outputs is required for computing the pervertex LDS size for * VS/TES when lowering NGG streamout. */ - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, nir->info.stage); + nir_assign_io_var_locations(nir, nir_var_shader_out); } } diff --git a/src/broadcom/vulkan/v3dv_pipeline.c b/src/broadcom/vulkan/v3dv_pipeline.c index fabc1c5607d..2561dc17f64 100644 --- a/src/broadcom/vulkan/v3dv_pipeline.c +++ b/src/broadcom/vulkan/v3dv_pipeline.c @@ -933,11 +933,8 @@ lower_fs_io(nir_shader *nir) NIR_PASS(_, nir, nir_lower_io_array_vars_to_elements_no_indirects, false); NIR_PASS(_, nir, nir_remove_dead_variables, nir_var_shader_out, NULL); - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, - MESA_SHADER_FRAGMENT); - - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, - MESA_SHADER_FRAGMENT); + nir_assign_io_var_locations(nir, nir_var_shader_in); + nir_assign_io_var_locations(nir, nir_var_shader_out); NIR_PASS(_, nir, nir_lower_io, nir_var_shader_in | nir_var_shader_out, type_size_vec4, 0); @@ -948,11 +945,8 @@ lower_gs_io(struct nir_shader *nir) { NIR_PASS(_, nir, nir_lower_io_array_vars_to_elements_no_indirects, false); - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, - MESA_SHADER_GEOMETRY); - - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, - MESA_SHADER_GEOMETRY); + nir_assign_io_var_locations(nir, nir_var_shader_in); + nir_assign_io_var_locations(nir, nir_var_shader_out); } static void @@ -960,11 +954,8 @@ lower_vs_io(struct nir_shader *nir) { NIR_PASS(_, nir, nir_lower_io_array_vars_to_elements_no_indirects, false); - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, - MESA_SHADER_VERTEX); - - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, - MESA_SHADER_VERTEX); + nir_assign_io_var_locations(nir, nir_var_shader_in); + nir_assign_io_var_locations(nir, nir_var_shader_out); /* FIXME: if we call nir_lower_io, we get a crash later. Likely because it * overlaps with v3d_nir_lower_io. Need further research though. diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h index 8535888ac1b..8f91f31174b 100644 --- a/src/compiler/nir/nir.h +++ b/src/compiler/nir/nir.h @@ -5123,10 +5123,7 @@ bool nir_lower_amul(nir_shader *shader, bool nir_lower_ubo_vec4(nir_shader *shader); void nir_sort_variables_by_location(nir_shader *shader, nir_variable_mode mode); -void nir_assign_io_var_locations(nir_shader *shader, - nir_variable_mode mode, - unsigned *size, - mesa_shader_stage stage); +void nir_assign_io_var_locations(nir_shader *shader, nir_variable_mode mode); bool nir_opt_clip_cull_const(nir_shader *shader); diff --git a/src/compiler/nir/nir_linking_helpers.c b/src/compiler/nir/nir_linking_helpers.c index 9cfe63d2d02..146cb8bccb2 100644 --- a/src/compiler/nir/nir_linking_helpers.c +++ b/src/compiler/nir/nir_linking_helpers.c @@ -1493,8 +1493,7 @@ nir_sort_variables_by_location(nir_shader *shader, nir_variable_mode mode) } void -nir_assign_io_var_locations(nir_shader *shader, nir_variable_mode mode, - unsigned *size, mesa_shader_stage stage) +nir_assign_io_var_locations(nir_shader *shader, nir_variable_mode mode) { unsigned location = 0; unsigned assigned_locations[VARYING_SLOT_TESS_MAX][2]; @@ -1508,16 +1507,17 @@ nir_assign_io_var_locations(nir_shader *shader, nir_variable_mode mode, bool last_partial = false; nir_foreach_variable_in_list(var, &io_vars) { const struct glsl_type *type = var->type; - if (nir_is_arrayed_io(var, stage)) { + if (nir_is_arrayed_io(var, shader->info.stage)) { assert(glsl_type_is_array(type)); type = glsl_get_array_element(type); } int base; - if (var->data.mode == nir_var_shader_in && stage == MESA_SHADER_VERTEX) + if (var->data.mode == nir_var_shader_in && + shader->info.stage == MESA_SHADER_VERTEX) base = VERT_ATTRIB_GENERIC0; else if (var->data.mode == nir_var_shader_out && - stage == MESA_SHADER_FRAGMENT) + shader->info.stage == MESA_SHADER_FRAGMENT) base = FRAG_RESULT_DATA0; else base = VARYING_SLOT_VAR0; @@ -1628,5 +1628,10 @@ nir_assign_io_var_locations(nir_shader *shader, nir_variable_mode mode, location++; exec_list_append(&shader->variables, &io_vars); - *size = location; + if (mode == nir_var_shader_in) + shader->num_inputs = location; + else if (mode == nir_var_shader_out) + shader->num_outputs = location; + else + UNREACHABLE("Unknown I/O variable mode"); } diff --git a/src/freedreno/ir3/ir3_shader.c b/src/freedreno/ir3/ir3_shader.c index 31523f89b2e..f2a8a66da58 100644 --- a/src/freedreno/ir3/ir3_shader.c +++ b/src/freedreno/ir3/ir3_shader.c @@ -710,13 +710,8 @@ ir3_shader_passthrough_tcs(struct ir3_shader *vs, unsigned patch_vertices) */ tcs->info.internal = false; - nir_assign_io_var_locations(tcs, nir_var_shader_in, - &tcs->num_inputs, - tcs->info.stage); - - nir_assign_io_var_locations(tcs, nir_var_shader_out, - &tcs->num_outputs, - tcs->info.stage); + nir_assign_io_var_locations(tcs, nir_var_shader_in); + nir_assign_io_var_locations(tcs, nir_var_shader_out); NIR_PASS(_, tcs, nir_lower_system_values); diff --git a/src/freedreno/vulkan/tu_clear_blit.cc b/src/freedreno/vulkan/tu_clear_blit.cc index cd5f9dc94a2..dd17cd42a21 100644 --- a/src/freedreno/vulkan/tu_clear_blit.cc +++ b/src/freedreno/vulkan/tu_clear_blit.cc @@ -767,8 +767,8 @@ compile_shader(struct tu_device *dev, struct nir_shader *nir, { nir->options = ir3_get_compiler_options(dev->compiler); - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, nir->info.stage); - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, nir->info.stage); + nir_assign_io_var_locations(nir, nir_var_shader_in); + nir_assign_io_var_locations(nir, nir_var_shader_out); struct ir3_const_allocations const_allocs = {}; if (consts > 0) diff --git a/src/freedreno/vulkan/tu_nir_lower_multiview.cc b/src/freedreno/vulkan/tu_nir_lower_multiview.cc index 2aa34cdde98..9f9734f56ae 100644 --- a/src/freedreno/vulkan/tu_nir_lower_multiview.cc +++ b/src/freedreno/vulkan/tu_nir_lower_multiview.cc @@ -91,14 +91,13 @@ tu_nir_lower_multiview(nir_shader *nir, uint32_t mask, struct tu_device *dev) /* Speculatively assign output locations so that we know num_outputs. We * will assign output locations for real after this pass. */ - unsigned num_outputs; - nir_assign_io_var_locations(nir, nir_var_shader_out, &num_outputs, MESA_SHADER_VERTEX); + nir_assign_io_var_locations(nir, nir_var_shader_out); /* In addition to the generic checks done by NIR, check that we don't * overflow VPC with the extra copies of gl_Position. */ if (!TU_DEBUG(NOMULTIPOS) && - num_views <= max_views_for_multipos && num_outputs + (num_views - 1) <= 32 && + num_views <= max_views_for_multipos && nir->num_outputs + (num_views - 1) <= 32 && nir_can_lower_multiview(nir, options)) { /* It appears that the multiview mask is ignored when multi-position * output is enabled, so we have to write 0 to inactive views ourselves. diff --git a/src/freedreno/vulkan/tu_shader.cc b/src/freedreno/vulkan/tu_shader.cc index 0786a07bd39..c58484e3ea8 100644 --- a/src/freedreno/vulkan/tu_shader.cc +++ b/src/freedreno/vulkan/tu_shader.cc @@ -2741,8 +2741,8 @@ tu_shader_create(struct tu_device *dev, &compute_sysval_options); } - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, nir->info.stage); - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, nir->info.stage); + nir_assign_io_var_locations(nir, nir_var_shader_in); + nir_assign_io_var_locations(nir, nir_var_shader_out); /* Gather information for transform feedback. This should be called after: * - nir_split_per_member_structs. diff --git a/src/gallium/frontends/lavapipe/lvp_pipeline.c b/src/gallium/frontends/lavapipe/lvp_pipeline.c index f73271c72ad..eeffc83b9c8 100644 --- a/src/gallium/frontends/lavapipe/lvp_pipeline.c +++ b/src/gallium/frontends/lavapipe/lvp_pipeline.c @@ -434,15 +434,14 @@ lvp_shader_lower(struct lvp_device *pdevice, nir_shader *nir, struct lvp_pipelin lvp_shader_optimize(nir); if (nir->info.stage != MESA_SHADER_VERTEX) - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, nir->info.stage); + nir_assign_io_var_locations(nir, nir_var_shader_in); else { nir->num_inputs = util_last_bit64(nir->info.inputs_read); nir_foreach_shader_in_variable(var, nir) { var->data.driver_location = var->data.location - VERT_ATTRIB_GENERIC0; } } - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, - nir->info.stage); + nir_assign_io_var_locations(nir, nir_var_shader_out); if (robustness) NIR_PASS(_, nir, lvp_nir_opt_robustness, pdevice, robustness); diff --git a/src/panfrost/vulkan/panvk_vX_shader.c b/src/panfrost/vulkan/panvk_vX_shader.c index b940b0efe83..5976e48c924 100644 --- a/src/panfrost/vulkan/panvk_vX_shader.c +++ b/src/panfrost/vulkan/panvk_vX_shader.c @@ -802,8 +802,7 @@ panvk_lower_nir(struct panvk_device *dev, nir_shader *nir, * number of varying loads, as this number is required during descriptor * lowering for v9+. */ if (stage == MESA_SHADER_FRAGMENT) { - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, - stage); + nir_assign_io_var_locations(nir, nir_var_shader_in); #if PAN_ARCH >= 9 shader->desc_info.max_varying_loads = nir->num_inputs; #endif @@ -899,12 +898,10 @@ panvk_lower_nir(struct panvk_device *dev, nir_shader *nir, } } else if (stage != MESA_SHADER_FRAGMENT) { /* Input varyings in fragment shader have been lowered early. */ - nir_assign_io_var_locations(nir, nir_var_shader_in, &nir->num_inputs, - stage); + nir_assign_io_var_locations(nir, nir_var_shader_in); } - nir_assign_io_var_locations(nir, nir_var_shader_out, &nir->num_outputs, - stage); + nir_assign_io_var_locations(nir, nir_var_shader_out); /* Needed to turn shader_temp into function_temp since the backend only * handles the latter for now.