From e395e57f073ac8c0ae70ff7a5d9e2b27ea909263 Mon Sep 17 00:00:00 2001 From: Emma Anholt Date: Thu, 2 Mar 2023 16:02:17 -0800 Subject: [PATCH] glsl: Delete constant-variables pass. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that we don't do GLSL IR constant propagation or constant folding, we can leave constant variable detection and handling to NIR. This also avoids some OOB array access in GLSL IR in a piglit test! Freedreno stats again look like noise: total instructions in shared programs: 2718412 -> 2718746 (0.01%) instructions in affected programs: 80497 -> 80831 (0.41%) total last-baryf in shared programs: 110015 -> 110510 (0.45%) last-baryf in affected programs: 35263 -> 35758 (1.40%) total full in shared programs: 189486 -> 189480 (<.01%) full in affected programs: 52 -> 46 (-11.54%) total constlen in shared programs: 494540 -> 494496 (<.01%) constlen in affected programs: 452 -> 408 (-9.73%) total sstall in shared programs: 198297 -> 197928 (-0.19%) sstall in affected programs: 3691 -> 3322 (-10.00%) total systall in shared programs: 432150 -> 431799 (-0.08%) systall in affected programs: 6070 -> 5719 (-5.78%) total waves in shared programs: 435098 -> 435110 (<.01%) waves in affected programs: 92 -> 104 (13.04%) Reviewed-by: Alyssa Rosenzweig Reviewed-by: Timothy Arceri Reviewed-by: Marek Olšák Part-of: --- src/amd/ci/radeonsi-raven-fails.txt | 1 - src/amd/ci/radeonsi-stoney-fails.txt | 1 - src/compiler/glsl/glsl_parser_extras.cpp | 4 - src/compiler/glsl/ir_optimization.h | 2 - src/compiler/glsl/meson.build | 1 - src/compiler/glsl/opt_constant_variable.cpp | 229 ------------------ src/compiler/glsl/test_optpass.cpp | 4 - .../drivers/llvmpipe/ci/llvmpipe-fails.txt | 2 - .../nouveau/ci/nouveau-gm206-fails.txt | 2 - .../drivers/r600/ci/r600-turks-fails.txt | 5 - .../drivers/radeonsi/ci/gfx10-navi10-fail.csv | 1 - .../radeonsi/ci/gfx10_3-navi21-fail.csv | 1 - .../radeonsi/ci/gfx11-gfx1100-fail.csv | 1 - .../radeonsi/ci/gfx8-polaris11-fail.csv | 1 - .../drivers/virgl/ci/virpipe-gl-fails.txt | 5 - 15 files changed, 260 deletions(-) delete mode 100644 src/compiler/glsl/opt_constant_variable.cpp diff --git a/src/amd/ci/radeonsi-raven-fails.txt b/src/amd/ci/radeonsi-raven-fails.txt index 36a1fc9349b..4562c4700db 100644 --- a/src/amd/ci/radeonsi-raven-fails.txt +++ b/src/amd/ci/radeonsi-raven-fails.txt @@ -81,7 +81,6 @@ spec@arb_shading_language_packing@execution@built-in-functions@fs-packhalf2x16,F spec@arb_shading_language_packing@execution@built-in-functions@vs-packhalf2x16,Fail spec@arb_sparse_buffer@buffer-data,Fail spec@arb_sparse_buffer@commit,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash spec@egl 1.4@eglterminate then unbind context,Fail spec@egl_chromium_sync_control@conformance,Fail spec@egl_chromium_sync_control@conformance@eglGetSyncValuesCHROMIUM_msc_and_sbc_test,Fail diff --git a/src/amd/ci/radeonsi-stoney-fails.txt b/src/amd/ci/radeonsi-stoney-fails.txt index 8fe0580d0eb..85e492bd22b 100644 --- a/src/amd/ci/radeonsi-stoney-fails.txt +++ b/src/amd/ci/radeonsi-stoney-fails.txt @@ -85,7 +85,6 @@ spec@arb_program_interface_query@arb_program_interface_query-getprogramresourcei spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgradcube,Fail spec@arb_shading_language_packing@execution@built-in-functions@fs-packhalf2x16,Fail spec@arb_shading_language_packing@execution@built-in-functions@vs-packhalf2x16,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash spec@arb_texture_compression@texwrap formats bordercolor-swizzled,Fail spec@arb_texture_compression@texwrap formats bordercolor-swizzled@GL_COMPRESSED_RGB- swizzled- border color only,Fail spec@arb_texture_compression@texwrap formats bordercolor-swizzled@GL_COMPRESSED_RGBA- swizzled- border color only,Fail diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp index b02fb3342fd..52e30f41b90 100644 --- a/src/compiler/glsl/glsl_parser_extras.cpp +++ b/src/compiler/glsl/glsl_parser_extras.cpp @@ -2398,10 +2398,6 @@ do_common_optimization(exec_list *ir, bool linked, OPT(do_dead_code_unlinked, ir); OPT(do_dead_code_local, ir); OPT(do_tree_grafting, ir); - if (linked) - OPT(do_constant_variable, ir); - else - OPT(do_constant_variable_unlinked, ir); OPT(do_minmax_prune, ir); OPT(do_rebalance_tree, ir); OPT(do_algebraic, ir, native_integers, options); diff --git a/src/compiler/glsl/ir_optimization.h b/src/compiler/glsl/ir_optimization.h index 9be8ac38fda..6cf6b2356d7 100644 --- a/src/compiler/glsl/ir_optimization.h +++ b/src/compiler/glsl/ir_optimization.h @@ -44,8 +44,6 @@ bool do_common_optimization(exec_list *ir, bool linked, bool do_rebalance_tree(exec_list *instructions); bool do_algebraic(exec_list *instructions, bool native_integers, const struct gl_shader_compiler_options *options); -bool do_constant_variable(exec_list *instructions); -bool do_constant_variable_unlinked(exec_list *instructions); bool do_dead_code(exec_list *instructions); bool do_dead_code_local(exec_list *instructions); bool do_dead_code_unlinked(exec_list *instructions); diff --git a/src/compiler/glsl/meson.build b/src/compiler/glsl/meson.build index 795678780bf..a65d2482b3c 100644 --- a/src/compiler/glsl/meson.build +++ b/src/compiler/glsl/meson.build @@ -214,7 +214,6 @@ files_libglsl = files( 'lower_vec_index_to_cond_assign.cpp', 'lower_vector_derefs.cpp', 'opt_algebraic.cpp', - 'opt_constant_variable.cpp', 'opt_dead_builtin_variables.cpp', 'opt_dead_code.cpp', 'opt_dead_code_local.cpp', diff --git a/src/compiler/glsl/opt_constant_variable.cpp b/src/compiler/glsl/opt_constant_variable.cpp deleted file mode 100644 index bbde9229950..00000000000 --- a/src/compiler/glsl/opt_constant_variable.cpp +++ /dev/null @@ -1,229 +0,0 @@ -/* - * Copyright © 2010 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_constant_variable.cpp - * - * Marks variables assigned a single constant value over the course - * of the program as constant. - * - * The goal here is to trigger further constant folding and then dead - * code elimination. This is common with vector/matrix constructors - * and calls to builtin functions. - */ - -#include "ir.h" -#include "ir_visitor.h" -#include "ir_optimization.h" -#include "compiler/glsl_types.h" -#include "util/hash_table.h" - -namespace { - -struct assignment_entry { - int assignment_count; - ir_variable *var; - ir_constant *constval; - bool our_scope; -}; - -class ir_constant_variable_visitor : public ir_hierarchical_visitor { -public: - using ir_hierarchical_visitor::visit; - using ir_hierarchical_visitor::visit_enter; - - virtual ir_visitor_status visit_enter(ir_dereference_variable *); - virtual ir_visitor_status visit(ir_variable *); - virtual ir_visitor_status visit_enter(ir_assignment *); - virtual ir_visitor_status visit_enter(ir_call *); - - struct hash_table *ht; -}; - -} /* unnamed namespace */ - -static struct assignment_entry * -get_assignment_entry(ir_variable *var, struct hash_table *ht) -{ - struct hash_entry *hte = _mesa_hash_table_search(ht, var); - struct assignment_entry *entry; - - if (hte) { - entry = (struct assignment_entry *) hte->data; - } else { - entry = (struct assignment_entry *) calloc(1, sizeof(*entry)); - entry->var = var; - _mesa_hash_table_insert(ht, var, entry); - } - - return entry; -} - -ir_visitor_status -ir_constant_variable_visitor::visit(ir_variable *ir) -{ - struct assignment_entry *entry = get_assignment_entry(ir, this->ht); - entry->our_scope = true; - return visit_continue; -} - -/* Skip derefs of variables so that we can detect declarations. */ -ir_visitor_status -ir_constant_variable_visitor::visit_enter(ir_dereference_variable *ir) -{ - (void)ir; - return visit_continue_with_parent; -} - -ir_visitor_status -ir_constant_variable_visitor::visit_enter(ir_assignment *ir) -{ - ir_constant *constval; - struct assignment_entry *entry; - - entry = get_assignment_entry(ir->lhs->variable_referenced(), this->ht); - assert(entry); - entry->assignment_count++; - - /* If there's more than one assignment, don't bother - we won't do anything - * with this variable anyway, and continuing just wastes memory cloning - * constant expressions. - */ - if (entry->assignment_count > 1) - return visit_continue; - - /* If it's already constant, don't do the work. */ - if (entry->var->constant_value) - return visit_continue; - - ir_variable *var = ir->whole_variable_written(); - if (!var) - return visit_continue; - - /* Ignore buffer variables, since the underlying storage is shared - * and we can't be sure that this variable won't be written by another - * thread. - */ - if (var->data.mode == ir_var_shader_storage || - var->data.mode == ir_var_shader_shared) - return visit_continue; - - constval = ir->rhs->constant_expression_value(ralloc_parent(ir)); - if (!constval) - return visit_continue; - - /* Mark this entry as having a constant assignment (if the - * assignment count doesn't go >1). do_constant_variable will fix - * up the variable with the constant value later. - */ - entry->constval = constval; - - return visit_continue; -} - -ir_visitor_status -ir_constant_variable_visitor::visit_enter(ir_call *ir) -{ - /* Mark any out parameters as assigned to */ - foreach_two_lists(formal_node, &ir->callee->parameters, - actual_node, &ir->actual_parameters) { - ir_rvalue *param_rval = (ir_rvalue *) actual_node; - ir_variable *param = (ir_variable *) formal_node; - - if (param->data.mode == ir_var_function_out || - param->data.mode == ir_var_function_inout) { - ir_variable *var = param_rval->variable_referenced(); - struct assignment_entry *entry; - - assert(var); - entry = get_assignment_entry(var, this->ht); - entry->assignment_count++; - } - - /* We don't know if the variable passed to this function has been - * assigned a value or if it is undefined, so for now we always assume - * it has been assigned a value. Once functions have been inlined any - * further potential optimisations will be taken care of. - */ - struct assignment_entry *entry; - entry = get_assignment_entry(param, this->ht); - entry->assignment_count++; - } - - /* Mark the return storage as having been assigned to */ - if (ir->return_deref != NULL) { - ir_variable *var = ir->return_deref->variable_referenced(); - struct assignment_entry *entry; - - assert(var); - entry = get_assignment_entry(var, this->ht); - entry->assignment_count++; - } - - return visit_continue; -} - -/** - * Does a copy propagation pass on the code present in the instruction stream. - */ -bool -do_constant_variable(exec_list *instructions) -{ - bool progress = false; - ir_constant_variable_visitor v; - - v.ht = _mesa_pointer_hash_table_create(NULL); - v.run(instructions); - - hash_table_foreach(v.ht, hte) { - struct assignment_entry *entry = (struct assignment_entry *) hte->data; - - if (entry->assignment_count == 1 && entry->constval && entry->our_scope) { - entry->var->constant_value = entry->constval; - progress = true; - } - hte->data = NULL; - free(entry); - } - _mesa_hash_table_destroy(v.ht, NULL); - - return progress; -} - -bool -do_constant_variable_unlinked(exec_list *instructions) -{ - bool progress = false; - - foreach_in_list(ir_instruction, ir, instructions) { - ir_function *f = ir->as_function(); - if (f) { - foreach_in_list(ir_function_signature, sig, &f->signatures) { - if (do_constant_variable(&sig->body)) - progress = true; - } - } - } - - return progress; -} diff --git a/src/compiler/glsl/test_optpass.cpp b/src/compiler/glsl/test_optpass.cpp index 701dc5ab600..6920a1ca52d 100644 --- a/src/compiler/glsl/test_optpass.cpp +++ b/src/compiler/glsl/test_optpass.cpp @@ -66,10 +66,6 @@ do_optimization(struct exec_list *ir, const char *optimization, return do_common_optimization(ir, int_0 != 0, options, true); } else if (strcmp(optimization, "do_algebraic") == 0) { return do_algebraic(ir, true, options); - } else if (strcmp(optimization, "do_constant_variable") == 0) { - return do_constant_variable(ir); - } else if (strcmp(optimization, "do_constant_variable_unlinked") == 0) { - return do_constant_variable_unlinked(ir); } else if (strcmp(optimization, "do_dead_code") == 0) { return do_dead_code(ir); } else if (strcmp(optimization, "do_dead_code_local") == 0) { diff --git a/src/gallium/drivers/llvmpipe/ci/llvmpipe-fails.txt b/src/gallium/drivers/llvmpipe/ci/llvmpipe-fails.txt index 8877938e4ec..3af2ea08f78 100644 --- a/src/gallium/drivers/llvmpipe/ci/llvmpipe-fails.txt +++ b/src/gallium/drivers/llvmpipe/ci/llvmpipe-fails.txt @@ -90,8 +90,6 @@ spec@arb_shader_image_load_store@execution@image-array-out-of-bounds-access-stor spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgrad,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash - spec@egl_khr_gl_image@egl_khr_gl_renderbuffer_image-clear-shared-image gl_depth_component24,Fail # No such file or directory (os error 2) diff --git a/src/gallium/drivers/nouveau/ci/nouveau-gm206-fails.txt b/src/gallium/drivers/nouveau/ci/nouveau-gm206-fails.txt index 31f381f9e96..23a572c2c87 100644 --- a/src/gallium/drivers/nouveau/ci/nouveau-gm206-fails.txt +++ b/src/gallium/drivers/nouveau/ci/nouveau-gm206-fails.txt @@ -455,8 +455,6 @@ spec@arb_shader_image_load_store@max-size@image2DMS max size test/8x8x16384x1,Fa spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgrad,Fail spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgradcube,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash - # "../nouveau/pushbuf.c:730: nouveau_pushbuf_data: Assertion `kref' failed." spec@arb_texture_buffer_object@render-no-bo,Crash diff --git a/src/gallium/drivers/r600/ci/r600-turks-fails.txt b/src/gallium/drivers/r600/ci/r600-turks-fails.txt index 0f82bd1f435..d77278ae063 100644 --- a/src/gallium/drivers/r600/ci/r600-turks-fails.txt +++ b/src/gallium/drivers/r600/ci/r600-turks-fails.txt @@ -768,11 +768,6 @@ spec@arb_depth_buffer_float@fbo-clear-formats stencil@GL_DEPTH32F_STENCIL8,Fail spec@arb_draw_indirect@gl_vertexid used with gldrawarraysindirect,Fail spec@arb_draw_indirect@gl_vertexid used with gldrawelementsindirect,Fail -# " intrinsic copy_deref (ssa_2, ssa_3) (dst_access=0, src_access=0) -# error: glsl_get_bare_type(dst->type) == glsl_get_bare_type(src->type) (../src/compiler/nir/nir_validate.c:643)" -# since ntt copy-deref optimization, probably. -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash - # "Testing level 3 # Probe at (0,8) # Expected: 219 diff --git a/src/gallium/drivers/radeonsi/ci/gfx10-navi10-fail.csv b/src/gallium/drivers/radeonsi/ci/gfx10-navi10-fail.csv index 8b763f88b2a..abea135bdb3 100644 --- a/src/gallium/drivers/radeonsi/ci/gfx10-navi10-fail.csv +++ b/src/gallium/drivers/radeonsi/ci/gfx10-navi10-fail.csv @@ -122,7 +122,6 @@ spec@khr_texture_compression_astc@sliced-3d-miptree-gl srgb-fp@sRGB decode full spec@khr_texture_compression_astc@sliced-3d-miptree-gles srgb-fp,Fail spec@khr_texture_compression_astc@sliced-3d-miptree-gles srgb-fp@sRGB decode full precision,Fail spec@oes_shader_io_blocks@compiler@layout-location-aliasing.vert,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash spec@ext_transform_feedback@builtin-varyings gl_culldistance,Fail # glcts failures diff --git a/src/gallium/drivers/radeonsi/ci/gfx10_3-navi21-fail.csv b/src/gallium/drivers/radeonsi/ci/gfx10_3-navi21-fail.csv index 41a1d98022e..4ec45402d1d 100644 --- a/src/gallium/drivers/radeonsi/ci/gfx10_3-navi21-fail.csv +++ b/src/gallium/drivers/radeonsi/ci/gfx10_3-navi21-fail.csv @@ -67,7 +67,6 @@ spec@arb_program_interface_query@arb_program_interface_query-getprogramresourcei spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgradcube,Fail spec@arb_shading_language_packing@execution@built-in-functions@fs-packhalf2x16,Fail spec@arb_shading_language_packing@execution@built-in-functions@vs-packhalf2x16,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash spec@arb_viewport_array@display-list,Fail spec@egl_ext_protected_content@conformance,Fail spec@ext_framebuffer_blit@fbo-blit-check-limits,Fail diff --git a/src/gallium/drivers/radeonsi/ci/gfx11-gfx1100-fail.csv b/src/gallium/drivers/radeonsi/ci/gfx11-gfx1100-fail.csv index 2f80c9476a7..70811072e6d 100644 --- a/src/gallium/drivers/radeonsi/ci/gfx11-gfx1100-fail.csv +++ b/src/gallium/drivers/radeonsi/ci/gfx11-gfx1100-fail.csv @@ -67,7 +67,6 @@ spec@arb_program_interface_query@arb_program_interface_query-getprogramresourcei spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgradcube,Fail spec@arb_shading_language_packing@execution@built-in-functions@fs-packhalf2x16,Fail spec@arb_shading_language_packing@execution@built-in-functions@vs-packhalf2x16,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash spec@egl_ext_protected_content@conformance,Fail spec@ext_framebuffer_blit@fbo-blit-check-limits,Fail spec@glsl-1.20@compiler@invalid-vec4-array-to-vec3-array-conversion.vert,Fail diff --git a/src/gallium/drivers/radeonsi/ci/gfx8-polaris11-fail.csv b/src/gallium/drivers/radeonsi/ci/gfx8-polaris11-fail.csv index ff710df0eb5..817a47e0072 100644 --- a/src/gallium/drivers/radeonsi/ci/gfx8-polaris11-fail.csv +++ b/src/gallium/drivers/radeonsi/ci/gfx8-polaris11-fail.csv @@ -91,7 +91,6 @@ spec@arb_query_buffer_object@qbo@query-GL_TIME_ELAPSED-ASYNC_CPU_READ_BEFORE-GL_ spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgradcube,Fail spec@arb_shading_language_packing@execution@built-in-functions@fs-packhalf2x16,Fail spec@arb_shading_language_packing@execution@built-in-functions@vs-packhalf2x16,Fail -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash spec@arb_texture_compression@texwrap formats bordercolor-swizzled,Fail spec@arb_texture_compression@texwrap formats bordercolor-swizzled@GL_COMPRESSED_RGB- swizzled- border color only,Fail spec@arb_texture_compression@texwrap formats bordercolor-swizzled@GL_COMPRESSED_RGBA- swizzled- border color only,Fail diff --git a/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt b/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt index 173339d7352..40e7b45e508 100644 --- a/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt +++ b/src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt @@ -384,11 +384,6 @@ spec@arb_shader_texture_image_samples@texturesamples@vs-usampler2dmsarray-2,Fail spec@arb_shader_texture_lod@execution@arb_shader_texture_lod-texgrad,Fail spec@arb_tessellation_shader@execution@gs-primitiveid-instanced,Fail -# " intrinsic copy_deref (ssa_2, ssa_3) (dst_access=0, src_access=0) -# error: glsl_get_bare_type(dst->type) == glsl_get_bare_type(src->type) (../src/compiler/nir/nir_validate.c:643)" -# since ntt copy-deref optimization, probably. -spec@arb_tessellation_shader@execution@tcs-tes-levels-out-of-bounds-write,Crash - spec@arb_texture_compression@texwrap formats bordercolor,Fail spec@arb_texture_compression@texwrap formats bordercolor-swizzled,Fail spec@arb_texture_compression@texwrap formats bordercolor-swizzled@GL_COMPRESSED_ALPHA- swizzled- border color only,Fail