From 6808ccf23a8b7538176201a8415be602cc88c755 Mon Sep 17 00:00:00 2001 From: Emma Anholt Date: Wed, 18 Jun 2025 14:17:07 -0700 Subject: [PATCH] mesa: Retire the OptimizeForAOS code. It's been unused for years. Back on the brw driver, it was used for a ~1% win on a VS-heavy workload, because you could hide instruction latency using a DP4 sequence instead of MADs for doing the MVP transform. Given that r300 and i915 don't seem to want it, and nobody has successfully ported it for crocus (see https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14277), just garbage collect the code at this point. Part-of: --- src/compiler/glsl/glsl_parser_extras.cpp | 3 - src/compiler/glsl/ir_optimization.h | 1 - src/compiler/glsl/meson.build | 1 - src/compiler/glsl/opt_flip_matrices.cpp | 123 ------------------ src/mesa/main/consts_exts.h | 9 -- src/mesa/main/ffvertex_prog.c | 39 ++---- src/mesa/program/prog_to_nir.c | 4 +- src/mesa/state_tracker/st_context.c | 8 -- src/mesa/state_tracker/st_nir.h | 2 +- .../st_nir_lower_position_invariant.c | 17 +-- 10 files changed, 18 insertions(+), 189 deletions(-) delete mode 100644 src/compiler/glsl/opt_flip_matrices.cpp diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp index 17b44839997..29feb58a18d 100644 --- a/src/compiler/glsl/glsl_parser_extras.cpp +++ b/src/compiler/glsl/glsl_parser_extras.cpp @@ -2507,9 +2507,6 @@ do_common_optimization(exec_list *ir, bool linked, OPT(do_if_simplification, ir); OPT(opt_flatten_nested_if_blocks, ir); - if (options->OptimizeForAOS && !linked) - OPT(opt_flip_matrices, ir); - OPT(do_dead_code_unlinked, ir); OPT(do_tree_grafting, ir); OPT(do_minmax_prune, ir); diff --git a/src/compiler/glsl/ir_optimization.h b/src/compiler/glsl/ir_optimization.h index 6d5c7f0fe3d..be461ca4246 100644 --- a/src/compiler/glsl/ir_optimization.h +++ b/src/compiler/glsl/ir_optimization.h @@ -44,7 +44,6 @@ bool do_algebraic(exec_list *instructions, bool native_integers, bool do_dead_code(exec_list *instructions); bool do_dead_code_local(exec_list *instructions); bool do_dead_code_unlinked(exec_list *instructions); -bool opt_flip_matrices(exec_list *instructions); bool do_lower_jumps(exec_list *instructions, bool pull_out_jumps = true, bool lower_continue = false); bool do_if_simplification(exec_list *instructions); bool opt_flatten_nested_if_blocks(exec_list *instructions); diff --git a/src/compiler/glsl/meson.build b/src/compiler/glsl/meson.build index 371740e736e..7fa3e894b7e 100644 --- a/src/compiler/glsl/meson.build +++ b/src/compiler/glsl/meson.build @@ -179,7 +179,6 @@ files_libglsl = files( 'opt_dead_builtin_variables.cpp', 'opt_dead_code.cpp', 'opt_flatten_nested_if_blocks.cpp', - 'opt_flip_matrices.cpp', 'opt_function_inlining.cpp', 'opt_if_simplification.cpp', 'opt_minmax.cpp', diff --git a/src/compiler/glsl/opt_flip_matrices.cpp b/src/compiler/glsl/opt_flip_matrices.cpp deleted file mode 100644 index 4b36d6ffdd3..00000000000 --- a/src/compiler/glsl/opt_flip_matrices.cpp +++ /dev/null @@ -1,123 +0,0 @@ -/* - * Copyright © 2013 Intel Corporation - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - * DEALINGS IN THE SOFTWARE. - */ - -/** - * \file opt_flip_matrices.cpp - * - * Convert (matrix * vector) operations to (vector * matrixTranspose), - * which can be done using dot products rather than multiplies and adds. - * On some hardware, this is more efficient. - * - * This currently only does the conversion for built-in matrices which - * already have transposed equivalents. Namely, gl_ModelViewProjectionMatrix - * and gl_TextureMatrix. - */ -#include "ir.h" -#include "ir_optimization.h" -#include "main/macros.h" - -namespace { -class matrix_flipper : public ir_hierarchical_visitor { -public: - matrix_flipper(exec_list *instructions) - { - progress = false; - mvp_transpose = NULL; - texmat_transpose = NULL; - - foreach_in_list(ir_instruction, ir, instructions) { - ir_variable *var = ir->as_variable(); - if (!var) - continue; - if (strcmp(var->name, "gl_ModelViewProjectionMatrixTranspose") == 0) - mvp_transpose = var; - if (strcmp(var->name, "gl_TextureMatrixTranspose") == 0) - texmat_transpose = var; - } - } - - ir_visitor_status visit_enter(ir_expression *ir); - - bool progress; - -private: - ir_variable *mvp_transpose; - ir_variable *texmat_transpose; -}; -} - -ir_visitor_status -matrix_flipper::visit_enter(ir_expression *ir) -{ - if (ir->operation != ir_binop_mul || - !glsl_type_is_matrix(ir->operands[0]->type) || - !glsl_type_is_vector(ir->operands[1]->type)) - return visit_continue; - - ir_variable *mat_var = ir->operands[0]->variable_referenced(); - if (!mat_var) - return visit_continue; - - if (mvp_transpose && - strcmp(mat_var->name, "gl_ModelViewProjectionMatrix") == 0) { -#ifndef NDEBUG - ir_dereference_variable *deref = ir->operands[0]->as_dereference_variable(); - assert(deref && deref->var == mat_var); -#endif - - void *mem_ctx = ralloc_parent(ir); - - ir->operands[0] = ir->operands[1]; - ir->operands[1] = new(mem_ctx) ir_dereference_variable(mvp_transpose); - - progress = true; - } else if (texmat_transpose && - strcmp(mat_var->name, "gl_TextureMatrix") == 0) { - ir_dereference_array *array_ref = ir->operands[0]->as_dereference_array(); - assert(array_ref != NULL); - ir_dereference_variable *var_ref = array_ref->array->as_dereference_variable(); - assert(var_ref && var_ref->var == mat_var); - - ir->operands[0] = ir->operands[1]; - ir->operands[1] = array_ref; - - var_ref->var = texmat_transpose; - - texmat_transpose->data.max_array_access = - MAX2(texmat_transpose->data.max_array_access, mat_var->data.max_array_access); - - progress = true; - } - - return visit_continue; -} - -bool -opt_flip_matrices(struct exec_list *instructions) -{ - matrix_flipper v(instructions); - - visit_list_elements(&v, instructions); - - return v.progress; -} diff --git a/src/mesa/main/consts_exts.h b/src/mesa/main/consts_exts.h index 3e7f7c9bb8a..63ca724f6e4 100644 --- a/src/mesa/main/consts_exts.h +++ b/src/mesa/main/consts_exts.h @@ -353,15 +353,6 @@ struct gl_shader_compiler_options GLuint MaxIfDepth; /**< Maximum nested IF blocks */ - /** - * Optimize code for array of structures backends. - * - * This is a proxy for: - * - preferring DP4 instructions (rather than MUL/MAD) for - * matrix * vector operations, such as position transformation. - */ - GLboolean OptimizeForAOS; - /** Clamp UBO and SSBO block indices so they don't go out-of-bounds. */ GLboolean ClampBlockIndicesToArrayBounds; diff --git a/src/mesa/main/ffvertex_prog.c b/src/mesa/main/ffvertex_prog.c index 6e6b969e077..eea5852e5ee 100644 --- a/src/mesa/main/ffvertex_prog.c +++ b/src/mesa/main/ffvertex_prog.c @@ -281,7 +281,6 @@ static void make_state_key( struct gl_context *ctx, struct state_key *key ) struct tnl_program { const struct state_key *state; struct gl_program_parameter_list *state_params; - GLboolean mvp_with_dp4; nir_builder *b; @@ -457,18 +456,11 @@ get_eye_position(struct tnl_program *p) if (!p->eye_position) { nir_def *pos = load_input_vec4(p, VERT_ATTRIB_POS); - if (p->mvp_with_dp4) { - nir_def *modelview[4]; - load_state_mat4(p, modelview, STATE_MODELVIEW_MATRIX, 0); - p->eye_position = - emit_matrix_transform_vec4(p->b, modelview, pos); - } else { - nir_def *modelview[4]; - load_state_mat4(p, modelview, - STATE_MODELVIEW_MATRIX_TRANSPOSE, 0); - p->eye_position = - emit_transpose_matrix_transform_vec4(p->b, modelview, pos); - } + nir_def *modelview[4]; + load_state_mat4(p, modelview, + STATE_MODELVIEW_MATRIX_TRANSPOSE, 0); + p->eye_position = + emit_transpose_matrix_transform_vec4(p->b, modelview, pos); } return p->eye_position; @@ -1216,17 +1208,11 @@ static void build_texture_transform( struct tnl_program *p ) if (p->state->unit[i].texmat_enabled) { nir_def *texmat[4]; - if (p->mvp_with_dp4) { - load_state_mat4(p, texmat, STATE_TEXTURE_MATRIX, i); - texcoord = - emit_matrix_transform_vec4(p->b, texmat, texcoord); - } else { - load_state_mat4(p, texmat, - STATE_TEXTURE_MATRIX_TRANSPOSE, i); - texcoord = - emit_transpose_matrix_transform_vec4(p->b, texmat, - texcoord); - } + load_state_mat4(p, texmat, + STATE_TEXTURE_MATRIX_TRANSPOSE, i); + texcoord = + emit_transpose_matrix_transform_vec4(p->b, texmat, + texcoord); } store_output_vec4(p, VARYING_SLOT_TEX0 + i, texcoord); @@ -1321,14 +1307,12 @@ static void build_tnl_program( struct tnl_program *p ) static nir_shader * create_new_program( const struct state_key *key, struct gl_program *program, - GLboolean mvp_with_dp4, const nir_shader_compiler_options *options) { struct tnl_program p; memset(&p, 0, sizeof(p)); p.state = key; - p.mvp_with_dp4 = mvp_with_dp4; program->Parameters = _mesa_new_parameter_list(); p.state_params = _mesa_new_parameter_list(); @@ -1349,7 +1333,7 @@ create_new_program( const struct state_key *key, nir_validate_shader(b.shader, "after generating ff-vertex shader"); /* Emit the MVP position transformation */ - NIR_PASS(_, b.shader, st_nir_lower_position_invariant, mvp_with_dp4, p.state_params); + NIR_PASS(_, b.shader, st_nir_lower_position_invariant, p.state_params); _mesa_add_separate_state_parameters(program, p.state_params); _mesa_free_parameter_list(p.state_params); @@ -1394,7 +1378,6 @@ _mesa_get_fixed_func_vertex_program(struct gl_context *ctx) nir_shader *s = create_new_program( &key, prog, - ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].OptimizeForAOS, options); prog->state.type = PIPE_SHADER_IR_NIR; diff --git a/src/mesa/program/prog_to_nir.c b/src/mesa/program/prog_to_nir.c index d5d8393af68..688282b9dd3 100644 --- a/src/mesa/program/prog_to_nir.c +++ b/src/mesa/program/prog_to_nir.c @@ -857,9 +857,7 @@ prog_to_nir(const struct gl_context *ctx, const struct gl_program *prog) /* ARB_vp: */ if (prog->arb.IsPositionInvariant) { - NIR_PASS(_, s, st_nir_lower_position_invariant, - ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].OptimizeForAOS, - prog->Parameters); + NIR_PASS(_, s, st_nir_lower_position_invariant, prog->Parameters); } /* Add OPTION ARB_fog_exp code */ diff --git a/src/mesa/state_tracker/st_context.c b/src/mesa/state_tracker/st_context.c index b562aed638a..3faa7e9b5dc 100644 --- a/src/mesa/state_tracker/st_context.c +++ b/src/mesa/state_tracker/st_context.c @@ -71,8 +71,6 @@ #include "compiler/glsl/glsl_parser_extras.h" #include "nir.h" -DEBUG_GET_ONCE_BOOL_OPTION(mesa_mvp_dp4, "MESA_MVP_DP4", false) - void st_invalidate_buffers(struct st_context *st) { @@ -833,12 +831,6 @@ st_create_context(gl_api api, struct pipe_context *pipe, if (pipe->screen->get_disk_shader_cache) ctx->Cache = pipe->screen->get_disk_shader_cache(pipe->screen); - /* XXX: need a capability bit in gallium to query if the pipe - * driver prefers DP4 or MUL/MAD for vertex transformation. - */ - if (debug_get_option_mesa_mvp_dp4()) - ctx->Const.ShaderCompilerOptions[MESA_SHADER_VERTEX].OptimizeForAOS = GL_TRUE; - if (pipe->screen->caps.invalidate_buffer) ctx->has_invalidate_buffer = true; diff --git a/src/mesa/state_tracker/st_nir.h b/src/mesa/state_tracker/st_nir.h index 794b6c60cfb..a7d707e7483 100644 --- a/src/mesa/state_tracker/st_nir.h +++ b/src/mesa/state_tracker/st_nir.h @@ -77,7 +77,7 @@ st_nir_state_variable_create(struct nir_shader *shader, bool st_nir_lower_fog(struct nir_shader *s, enum gl_fog_mode fog_mode, struct gl_program_parameter_list *paramList); -bool st_nir_lower_position_invariant(struct nir_shader *s, bool aos, +bool st_nir_lower_position_invariant(struct nir_shader *s, struct gl_program_parameter_list *paramList); bool st_nir_unlower_io_to_vars(struct nir_shader *nir); diff --git a/src/mesa/state_tracker/st_nir_lower_position_invariant.c b/src/mesa/state_tracker/st_nir_lower_position_invariant.c index dcf9b9af330..0dabf32f8a4 100644 --- a/src/mesa/state_tracker/st_nir_lower_position_invariant.c +++ b/src/mesa/state_tracker/st_nir_lower_position_invariant.c @@ -17,7 +17,7 @@ * both FF VS and ARB_vp. */ bool -st_nir_lower_position_invariant(struct nir_shader *s, bool aos, +st_nir_lower_position_invariant(struct nir_shader *s, struct gl_program_parameter_list *paramList) { assert(s->info.io_lowered); @@ -27,7 +27,7 @@ st_nir_lower_position_invariant(struct nir_shader *s, bool aos, nir_def *mvp[4]; for (int i = 0; i < 4; i++) { gl_state_index16 tokens[STATE_LENGTH] = { - aos ? STATE_MVP_MATRIX : STATE_MVP_MATRIX_TRANSPOSE, 0, i, i}; + STATE_MVP_MATRIX_TRANSPOSE, 0, i, i}; nir_variable *var = st_nir_state_variable_create(s, glsl_vec4_type(), tokens); _mesa_add_state_reference(paramList, tokens); mvp[i] = nir_load_var(&b, var); @@ -37,16 +37,9 @@ st_nir_lower_position_invariant(struct nir_shader *s, bool aos, nir_def *in_pos = nir_load_input(&b, 4, 32, nir_imm_int(&b, 0), .io_semantics.location = VERT_ATTRIB_POS); - if (aos) { - nir_def *chans[4]; - for (int i = 0; i < 4; i++) - chans[i] = nir_fdot4(&b, mvp[i], in_pos); - result = nir_vec4(&b, chans[0], chans[1], chans[2], chans[3]); - } else { - result = nir_fmul(&b, mvp[0], nir_channel(&b, in_pos, 0)); - for (int i = 1; i < 4; i++) - result = nir_fmad(&b, mvp[i], nir_channel(&b, in_pos, i), result); - } + result = nir_fmul(&b, mvp[0], nir_channel(&b, in_pos, 0)); + for (int i = 1; i < 4; i++) + result = nir_fmad(&b, mvp[i], nir_channel(&b, in_pos, i), result); nir_store_output(&b, result, nir_imm_int(&b, 0), .io_semantics.location = VARYING_SLOT_POS);