linker: Accurately track gl_uniform_block::stageref

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:

    ES31-CTS.core.geometry_shader.program_resource.program_resource
    ES32-CTS.core.geometry_shader.program_resource.program_resource
    ESEXT-CTS.geometry_shader.program_resource.program_resource
    piglit.gl45-cts.geometry_shader.program_resource.program_resource

However, it makes quite a few more fail:

    ES31-CTS.functional.program_interface_query.buffer_variable.random.6
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.compute.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.separable_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_fragment_only_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_geo_fragment_only_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_geo_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_fragment_only_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_geo_fragment_only_fragment.unnamed_block.float
    ES31-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_geo_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.random.6
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.compute.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.separable_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_fragment_only_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_geo_fragment_only_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_geo_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_fragment_only_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_geo_fragment_only_fragment.unnamed_block.float
    ES32-CTS.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_geo_fragment.unnamed_block.float

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.

After discussions with the OpenGL ES group in Khronos, we believe that
Mesa's behavior is correct.  I will provide patches to the CTS tests
to Khronos.

Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
index d3a77d5..54adbdf 100644 (file)
--- 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 (file)
--- 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];
          }
       }