diff options
author | Ian Romanick <[email protected]> | 2016-11-07 15:59:35 -0800 |
---|---|---|
committer | Ian Romanick <[email protected]> | 2016-11-09 12:47:51 -0800 |
commit | 084105c213dbf4c309453b33a966aac6ae9242f3 (patch) | |
tree | 9b44424328c9ab19b3fb77ad0ac7737bcacd0d9f /src/compiler | |
parent | 392fabcfee489c294dad5bed0bb57a2c61322e4d (diff) |
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 <[email protected]>
Reviewed-by: Nicolai Hähnle <[email protected]>
Reviewed-by: Kenneth Graunke <[email protected]>
Diffstat (limited to 'src/compiler')
-rw-r--r-- | src/compiler/glsl/link_uniforms.cpp | 65 | ||||
-rw-r--r-- | 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 d3a77d58c3b..54adbdf38ff 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 aceea8d520e..693a50b20d5 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]; } } |