glsl: Delete constant-variables pass.
authorEmma Anholt <emma@anholt.net>
Fri, 3 Mar 2023 00:02:17 +0000 (16:02 -0800)
committerMarge Bot <emma+marge@anholt.net>
Wed, 15 Mar 2023 03:29:19 +0000 (03:29 +0000)
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 <alyssa.rosenzweig@collabora.com>
Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com>
Reviewed-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/21751>

15 files changed:
src/amd/ci/radeonsi-raven-fails.txt
src/amd/ci/radeonsi-stoney-fails.txt
src/compiler/glsl/glsl_parser_extras.cpp
src/compiler/glsl/ir_optimization.h
src/compiler/glsl/meson.build
src/compiler/glsl/opt_constant_variable.cpp [deleted file]
src/compiler/glsl/test_optpass.cpp
src/gallium/drivers/llvmpipe/ci/llvmpipe-fails.txt
src/gallium/drivers/nouveau/ci/nouveau-gm206-fails.txt
src/gallium/drivers/r600/ci/r600-turks-fails.txt
src/gallium/drivers/radeonsi/ci/gfx10-navi10-fail.csv
src/gallium/drivers/radeonsi/ci/gfx10_3-navi21-fail.csv
src/gallium/drivers/radeonsi/ci/gfx11-gfx1100-fail.csv
src/gallium/drivers/radeonsi/ci/gfx8-polaris11-fail.csv
src/gallium/drivers/virgl/ci/virpipe-gl-fails.txt

index 36a1fc9..4562c47 100644 (file)
@@ -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
index 8fe0580..85e492b 100644 (file)
@@ -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
index b02fb33..52e30f4 100644 (file)
@@ -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);
index 9be8ac3..6cf6b23 100644 (file)
@@ -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);
index 7956787..a65d248 100644 (file)
@@ -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 (file)
index bbde922..0000000
+++ /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;
-}
index 701dc5a..6920a1c 100644 (file)
@@ -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) {
index 8877938..3af2ea0 100644 (file)
@@ -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)
index 31f381f..23a572c 100644 (file)
@@ -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
 
index 0f82bd1..d77278a 100644 (file)
@@ -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
index 8b763f8..abea135 100644 (file)
@@ -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
index 41a1d98..4ec4540 100644 (file)
@@ -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
index 2f80c94..7081107 100644 (file)
@@ -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
index ff710df..817a47e 100644 (file)
@@ -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
index 173339d..40e7b45 100644 (file)
@@ -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