[Mesa-dev] [PATCH 03/58] glsl: use linked_shaders bitmask to iterate stages for subroutine fields

[Ian Romanick]

With or without Emil's suggestions, this patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

On 11/20/2016 05:28 AM, Timothy Arceri wrote:
> This should be faster than looping over every stage and null checking, but
> will also make the code a bit cleaner when we switch to getting more fields
> from gl_program rather than from gl_linked_shader as we can just copy the
> pointer and not need to worry about null checking then copying.

If you're going to make even weak, speculative performance "claims," you
might want to include some sort of data.  Even 'size' of a release build
before and after.  It's not really a big deal for this patch, but
sometimes the results can surprise you... and lead to other discoveries.

> ---
>  src/compiler/glsl/link_uniforms.cpp | 12 ++++++------
>  src/compiler/glsl/linker.cpp        | 34 +++++++++++++++++-----------------
>  2 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
> index f271093..1c16f64 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -1138,10 +1138,10 @@ link_setup_uniform_remap_tables(struct gl_context *ctx,
>        const unsigned entries =
>           MAX2(1, prog->data->UniformStorage[i].array_elements);
>  
> -      for (unsigned j = 0; j < MESA_SHADER_STAGES; j++) {
> +      unsigned mask = prog->data->linked_stages;
> +      while (mask) {
> +         const int j = u_bit_scan(&mask);
>           struct gl_linked_shader *sh = prog->_LinkedShaders[j];
> -         if (!sh)
> -            continue;
>  
>           if (!prog->data->UniformStorage[i].opaque[j].active)
>              continue;
> @@ -1170,10 +1170,10 @@ link_setup_uniform_remap_tables(struct gl_context *ctx,
>        const unsigned entries =
>           MAX2(1, prog->data->UniformStorage[i].array_elements);
>  
> -      for (unsigned j = 0; j < MESA_SHADER_STAGES; j++) {
> +      unsigned mask = prog->data->linked_stages;
> +      while (mask) {
> +         const int j = u_bit_scan(&mask);
>           struct gl_linked_shader *sh = prog->_LinkedShaders[j];
> -         if (!sh)
> -            continue;
>  
>           if (!prog->data->UniformStorage[i].opaque[j].active)
>              continue;
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index ba9d97a..36616d8 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -3134,11 +3134,10 @@ check_resources(struct gl_context *ctx, struct gl_shader_program *prog)
>  static void
>  link_calculate_subroutine_compat(struct gl_shader_program *prog)
>  {
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +      const int i = u_bit_scan(&mask);
>        struct gl_linked_shader *sh = prog->_LinkedShaders[i];
> -      int count;
> -      if (!sh)
> -         continue;
>  
>        for (unsigned j = 0; j < sh->NumSubroutineUniformRemapTable; j++) {
>           if (sh->SubroutineUniformRemapTable[j] == INACTIVE_UNIFORM_EXPLICIT_LOCATION)
> @@ -3150,7 +3149,7 @@ link_calculate_subroutine_compat(struct gl_shader_program *prog)
>              continue;
>  
>           sh->NumSubroutineUniforms++;
> -         count = 0;
> +         int count = 0;
>           if (sh->NumSubroutineFunctions == 0) {
>              linker_error(prog, "subroutine uniform %s defined but no valid functions found\n", uni->type->name);
>              continue;
> @@ -3172,7 +3171,9 @@ link_calculate_subroutine_compat(struct gl_shader_program *prog)
>  static void
>  check_subroutine_resources(struct gl_shader_program *prog)
>  {
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +      const int i = u_bit_scan(&mask);
>        struct gl_linked_shader *sh = prog->_LinkedShaders[i];
>  
>        if (sh) {
> @@ -3374,7 +3375,9 @@ check_explicit_uniform_locations(struct gl_context *ctx,
>     }
>  
>     unsigned entries_total = 0;
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +      const int i = u_bit_scan(&mask);
>        struct gl_linked_shader *sh = prog->_LinkedShaders[i];
>  
>        if (!sh)
> @@ -4308,14 +4311,12 @@ build_program_resource_list(struct gl_context *ctx,
>        }
>     }
>  
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = shProg->data->linked_stages;
> +   while (mask) {
> +      const int i = u_bit_scan(&mask);
>        struct gl_linked_shader *sh = shProg->_LinkedShaders[i];
> -      GLuint type;
>  
> -      if (!sh)
> -         continue;
> -
> -      type = _mesa_shader_stage_to_subroutine((gl_shader_stage)i);
> +      GLuint type = _mesa_shader_stage_to_subroutine((gl_shader_stage)i);
>        for (unsigned j = 0; j < sh->NumSubroutineFunctions; j++) {
>           if (!add_program_resource(shProg, resource_set,
>                                     type, &sh->SubroutineFunctions[j], 0))
> @@ -4363,12 +4364,11 @@ validate_sampler_array_indexing(struct gl_context *ctx,
>  static void
>  link_assign_subroutine_types(struct gl_shader_program *prog)
>  {
> -   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> +   unsigned mask = prog->data->linked_stages;
> +   while (mask) {
> +      const int i = u_bit_scan(&mask);
>        gl_linked_shader *sh = prog->_LinkedShaders[i];
>  
> -      if (sh == NULL)
> -         continue;
> -
>        sh->MaxSubroutineFunctionIndex = 0;
>        foreach_in_list(ir_instruction, node, sh->ir) {
>           ir_function *fn = node->as_function();
>