From ed0f39360701405198ad582eb4b79d737add83a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= Date: Fri, 30 May 2025 06:17:45 -0400 Subject: [PATCH] ac/nir/lower_ngg: rename clip_cull_dist_mask and use it correctly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We incorrectly used it to determine whether the shader should cull, which luckily had no effect because it wasn't used everywhere. cull_clipdist_mask should be used instead, which also reflects whether clip planes are enabled in GL. clip_cull_dist_mask is renamed to export_clipdist_mask to make it clear. Reviewed-by: Pierre-Eric Pelloux-Prayer Reviewed-by: Timur Kristóf Part-of: --- src/amd/common/nir/ac_nir.h | 7 ++++--- src/amd/common/nir/ac_nir_lower_ngg.c | 11 +++++------ src/amd/common/nir/ac_nir_lower_ngg_gs.c | 2 +- src/amd/vulkan/radv_shader.c | 8 ++++---- src/gallium/drivers/radeonsi/si_shader.c | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/amd/common/nir/ac_nir.h b/src/amd/common/nir/ac_nir.h index 81f01649517..b073f91aea5 100644 --- a/src/amd/common/nir/ac_nir.h +++ b/src/amd/common/nir/ac_nir.h @@ -165,16 +165,17 @@ typedef struct { unsigned max_workgroup_size; unsigned wave_size; - uint8_t clip_cull_dist_mask; + /* The mask of clip and cull distances that the shader should export. */ + uint8_t export_clipdist_mask; /* The mask of clip and cull distances that the shader should cull against. * If no clip and cull distance outputs are present, it will load clip planes and cull * either against CLIP_VERTEX or POS. */ uint8_t cull_clipdist_mask; bool write_pos_to_clipvertex; - /* Remove clip/cull distance components that are missing in clip_cull_dist_mask, improving + /* Remove clip/cull distance components that are missing in export_clipdist_mask, improving * throughput by up to 50% (3 pos exports -> 2 pos exports). The caller shouldn't set no-op - * components (>= 0) in clip_cull_dist_mask to remove those completely. No-op components + * components (>= 0) in export_clipdist_mask to remove those completely. No-op components * should be determined by nir_opt_clip_cull_const before this. */ bool pack_clip_cull_distances; diff --git a/src/amd/common/nir/ac_nir_lower_ngg.c b/src/amd/common/nir/ac_nir_lower_ngg.c index 4d2affeab4f..139fa22c5bf 100644 --- a/src/amd/common/nir/ac_nir_lower_ngg.c +++ b/src/amd/common/nir/ac_nir_lower_ngg.c @@ -359,8 +359,7 @@ remove_culling_shader_output(nir_builder *b, nir_intrinsic_instr *intrin, void * unsigned base = io_sem.location == VARYING_SLOT_CLIP_DIST1 ? 4 : 0; base += component; - /* valid clipdist component mask */ - unsigned mask = (s->options->clip_cull_dist_mask >> base) & writemask; + unsigned mask = (s->options->cull_clipdist_mask >> base) & writemask; u_foreach_bit(i, mask) { add_clipdist_bit(b, nir_channel(b, store_val, i), base + i, s->clipdist_neg_mask_var); @@ -369,7 +368,8 @@ remove_culling_shader_output(nir_builder *b, nir_intrinsic_instr *intrin, void * break; } case VARYING_SLOT_CLIP_VERTEX: - ac_nir_store_var_components(b, s->clip_vertex_var, store_val, component, writemask); + if (s->options->cull_clipdist_mask) + ac_nir_store_var_components(b, s->clip_vertex_var, store_val, component, writemask); break; default: break; @@ -1040,8 +1040,7 @@ add_deferred_attribute_culling(nir_builder *b, nir_cf_list *original_extracted_c /* Relative patch ID is a special case because it doesn't need an extra dword, repack separately. */ s->repacked_rel_patch_id = nir_local_variable_create(impl, glsl_uint_type(), "repacked_rel_patch_id"); - if (s->options->clip_cull_dist_mask || - s->options->cull_clipdist_mask) { + if (s->options->cull_clipdist_mask) { s->clip_vertex_var = nir_local_variable_create(impl, glsl_vec4_type(), "clip_vertex"); s->clipdist_neg_mask_var = @@ -1746,7 +1745,7 @@ ac_nir_lower_ngg_nogs(nir_shader *shader, const ac_nir_lower_ngg_options *option } ac_nir_export_position(b, options->hw_info->gfx_level, - options->clip_cull_dist_mask, + options->export_clipdist_mask, options->write_pos_to_clipvertex, options->pack_clip_cull_distances, !options->has_param_exports, diff --git a/src/amd/common/nir/ac_nir_lower_ngg_gs.c b/src/amd/common/nir/ac_nir_lower_ngg_gs.c index 12bb1b72679..18c6649dcac 100644 --- a/src/amd/common/nir/ac_nir_lower_ngg_gs.c +++ b/src/amd/common/nir/ac_nir_lower_ngg_gs.c @@ -468,7 +468,7 @@ ngg_gs_emit_output(nir_builder *b, nir_def *max_num_out_vtx, nir_def *max_num_ou export_outputs &= ~VARYING_BIT_LAYER; ac_nir_export_position(b, s->options->hw_info->gfx_level, - s->options->clip_cull_dist_mask, + s->options->export_clipdist_mask, s->options->write_pos_to_clipvertex, s->options->pack_clip_cull_distances, !s->options->has_param_exports, diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c index 63bcecaf651..071af4c9c44 100644 --- a/src/amd/vulkan/radv_shader.c +++ b/src/amd/vulkan/radv_shader.c @@ -791,7 +791,7 @@ radv_lower_ngg(struct radv_device *device, struct radv_shader_stage *ngg_stage, options.hw_info = &pdev->info; options.max_workgroup_size = info->workgroup_size; options.wave_size = info->wave_size; - options.clip_cull_dist_mask = info->outinfo.clip_dist_mask | info->outinfo.cull_dist_mask; + options.export_clipdist_mask = info->outinfo.clip_dist_mask | info->outinfo.cull_dist_mask; options.vs_output_param_offset = info->outinfo.vs_output_param_offset; options.has_param_exports = info->outinfo.param_exports || info->outinfo.prim_param_exports; options.can_cull = nir->info.stage != MESA_SHADER_GEOMETRY && info->has_ngg_culling; @@ -825,9 +825,9 @@ radv_lower_ngg(struct radv_device *device, struct radv_shader_stage *ngg_stage, unsigned hw_workgroup_size = ALIGN(info->workgroup_size, info->wave_size); bool scratch_ring = false; - NIR_PASS(_, nir, ac_nir_lower_ngg_mesh, &pdev->info, options.clip_cull_dist_mask, - options.vs_output_param_offset, options.has_param_exports, &scratch_ring, info->wave_size, - hw_workgroup_size, gfx_state->has_multiview_view_index, info->ms.has_query, pdev->mesh_fast_launch_2); + NIR_PASS(_, nir, ac_nir_lower_ngg_mesh, &pdev->info, options.export_clipdist_mask, options.vs_output_param_offset, + options.has_param_exports, &scratch_ring, info->wave_size, hw_workgroup_size, + gfx_state->has_multiview_view_index, info->ms.has_query, pdev->mesh_fast_launch_2); ngg_stage->info.ms.needs_ms_scratch_ring = scratch_ring; } else { unreachable("invalid SW stage passed to radv_lower_ngg"); diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index d048183d61b..972eceaba6f 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -1131,7 +1131,7 @@ static void si_lower_ngg(struct si_shader *shader, nir_shader *nir, .disable_streamout = !shader->info.num_streamout_vec4s, .vs_output_param_offset = temp_info->vs_output_param_offset, .has_param_exports = shader->info.nr_param_exports, - .clip_cull_dist_mask = clip_cull_dist_mask, + .export_clipdist_mask = clip_cull_dist_mask, .kill_pointsize = key->ge.opt.kill_pointsize, .kill_layer = key->ge.opt.kill_layer, .force_vrs = sel->screen->options.vrs2x2,