With or without Emil's suggestions, this patch is

Reviewed-by: Ian Romanick <ian.d.roman...@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();
> 

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to