[Mesa-dev] [PATCH 4/4] linker: Accurately track gl_uniform_block::stageref

[Ian Romanick]

From: Ian Romanick <ian.d.romanick at intel.com>

As the linked per-stage shaders are processed, mark any block that has a
field that is accessed as referenced.  When combining all the linked
shaders, combine the per-stage stageref masks.

This fixes a number of GLES CTS tests including
ESEXT-CTS.geometry_shader.program_resource.program_resource.  However,
it makes quite a few more fail.  I have diagnosed the failures, but I'm
not sure whether we or the tests are wrong.  After optimizations are
applied, all of the tests are of the form:

    buffer X {
        float f;
    } x;

    void main()
    {
        x.f = x.f;
    }

The test then queries that x is referenced by that shader stage.  We
eliminate the assignment of x.f to itself, and that removes the last
reference to x.  We report that x is not referenced, and the test fails.
I do not know whether or not we are allowed to eliminate that assignment
of x.f to itself.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
---
 src/compiler/glsl/link_uniforms.cpp | 65 +++++++++++++++++++++++++++++++++----
 src/compiler/glsl/linker.cpp        |  3 +-
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
index d70614f..29cd24c 100644
--- a/src/compiler/glsl/link_uniforms.cpp
+++ b/src/compiler/glsl/link_uniforms.cpp
@@ -28,6 +28,7 @@
 #include "glsl_symbol_table.h"
 #include "program.h"
 #include "util/string_to_uint_map.h"
+#include "ir_variable_refcount.h"
 
 /**
  * \file link_uniforms.cpp
@@ -877,6 +878,15 @@ public:
    unsigned shader_shadow_samplers;
 };
 
+static bool
+variable_is_referenced(ir_variable_refcount_visitor &v, ir_variable *var)
+{
+   ir_variable_refcount_entry *const entry = v.get_variable_entry(var);
+
+   return entry->referenced_count > 0;
+
+}
+
 /**
  * Walks the IR and update the references to uniform blocks in the
  * ir_variables to point at linked shader's list (previously, they
@@ -884,8 +894,13 @@ public:
  * shaders).
  */
 static void
-link_update_uniform_buffer_variables(struct gl_linked_shader *shader)
+link_update_uniform_buffer_variables(struct gl_linked_shader *shader,
+                                     unsigned stage)
 {
+   ir_variable_refcount_visitor v;
+
+   v.run(shader->ir);
+
    foreach_in_list(ir_instruction, node, shader->ir) {
       ir_variable *const var = node->as_variable();
 
@@ -895,7 +910,44 @@ link_update_uniform_buffer_variables(struct gl_linked_shader *shader)
       assert(var->data.mode == ir_var_uniform ||
              var->data.mode == ir_var_shader_storage);
 
+      unsigned num_blocks = var->data.mode == ir_var_uniform ?
+         shader->NumUniformBlocks : shader->NumShaderStorageBlocks;
+      struct gl_uniform_block **blks = var->data.mode == ir_var_uniform ?
+         shader->UniformBlocks : shader->ShaderStorageBlocks;
+
       if (var->is_interface_instance()) {
+         if (variable_is_referenced(v, var)) {
+            /* Since this is an interface instance, the instance type will be
+             * same as the array-stripped variable type.  If the variable type
+             * is an array, then the block names will be suffixed with [0]
+             * through [n-1].  Unlike for non-interface instances, there will
+             * not be structure types here, so the only name sentinel that we
+             * have to worry about is [.
+             */
+            assert(var->type->without_array() == var->get_interface_type());
+            const char sentinel = var->type->is_array() ? '[' : '\0';
+
+            const ptrdiff_t len = strlen(var->get_interface_type()->name);
+            for (unsigned i = 0; i < num_blocks; i++) {
+               const char *const begin = blks[i]->Name;
+               const char *const end = strchr(begin, sentinel);
+
+               if (end == NULL)
+                  continue;
+
+               if (len != (end - begin))
+                  continue;
+
+               /* Even when a match is found, do not "break" here.  This could
+                * be an array of instances, and all elements of the array need
+                * to be marked as referenced.
+                */
+               if (strncmp(begin, var->get_interface_type()->name, len) == 0) {
+                  blks[i]->stageref |= 1U << stage;
+               }
+            }
+         }
+
          var->data.location = 0;
          continue;
       }
@@ -910,11 +962,6 @@ link_update_uniform_buffer_variables(struct gl_linked_shader *shader)
          sentinel = '[';
       }
 
-      unsigned num_blocks = var->data.mode == ir_var_uniform ?
-         shader->NumUniformBlocks : shader->NumShaderStorageBlocks;
-      struct gl_uniform_block **blks = var->data.mode == ir_var_uniform ?
-         shader->UniformBlocks : shader->ShaderStorageBlocks;
-
       const unsigned l = strlen(var->name);
       for (unsigned i = 0; i < num_blocks; i++) {
          for (unsigned j = 0; j < blks[i]->NumUniforms; j++) {
@@ -935,6 +982,10 @@ link_update_uniform_buffer_variables(struct gl_linked_shader *shader)
 
             if (found) {
                var->data.location = j;
+
+               if (variable_is_referenced(v, var))
+                  blks[i]->stageref |= 1U << stage;
+
                break;
             }
          }
@@ -1257,7 +1308,7 @@ link_assign_uniform_locations(struct gl_shader_program *prog,
       memset(sh->SamplerUnits, 0, sizeof(sh->SamplerUnits));
       memset(sh->ImageUnits, 0, sizeof(sh->ImageUnits));
 
-      link_update_uniform_buffer_variables(sh);
+      link_update_uniform_buffer_variables(sh, i);
 
       /* Reset various per-shader target counts.
        */
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index aceea8d..693a50b 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -1191,11 +1191,10 @@ interstage_cross_validate_uniform_blocks(struct gl_shader_program *prog,
          if (stage_index != -1) {
             struct gl_linked_shader *sh = prog->_LinkedShaders[i];
 
-            blks[j].stageref |= (1 << i);
-
             struct gl_uniform_block **sh_blks = validate_ssbo ?
                sh->ShaderStorageBlocks : sh->UniformBlocks;
 
+            blks[j].stageref |= sh_blks[stage_index]->stageref;
             sh_blks[stage_index] = &blks[j];
          }
       }
-- 
2.5.5