[Mesa-dev] [PATCH 3/3] glsl/linker: validate explicit locations for SSO programs

[Timothy Arceri]

On 20/10/17 21:46, Iago Toral Quiroga wrote:
> ---
>   src/compiler/glsl/link_varyings.cpp | 53 +++++++++++++++++++++++++++++++++++++
>   src/compiler/glsl/link_varyings.h   |  4 +++
>   src/compiler/glsl/linker.cpp        |  6 +++++
>   3 files changed, 63 insertions(+)
> 
> diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
> index dffb4f98df..d7e407184d 100644
> --- a/src/compiler/glsl/link_varyings.cpp
> +++ b/src/compiler/glsl/link_varyings.cpp
> @@ -599,6 +599,59 @@ validate_explicit_variable_location(struct gl_context *ctx,
>   }
>   
>   /**
> + * Validate explicit locations for SSO programs. For non-SSO programs this
> + * is alreadty done in cross_validate_outputs_to_inputs().
> + */
> +void
> +validate_sso_explicit_locations(struct gl_context *ctx,
> +                                struct gl_shader_program *prog)
> +{
> +   assert(prog->SeparateShader);
> +   struct explicit_location_info explicit_locations_in[MAX_VARYINGS_INCL_PATCH][4];
> +   struct explicit_location_info explicit_locations_out[MAX_VARYINGS_INCL_PATCH][4];
> +
> +   for (unsigned i = 0; i <= MESA_SHADER_FRAGMENT; i++) {
> +      if (prog->_LinkedShaders[i] == NULL)
> +         continue;
> +
> +      gl_linked_shader *sh = prog->_LinkedShaders[i];
> +
> +      memset(explicit_locations_in, 0, sizeof(explicit_locations_in));
> +      memset(explicit_locations_out, 0, sizeof(explicit_locations_out));
> +
> +      foreach_in_list(ir_instruction, node, sh->ir) {
> +         ir_variable *const var = node->as_variable();
> +
> +         if (var == NULL ||
> +             !var->data.explicit_location ||
> +             var->data.location < VARYING_SLOT_VAR0 ||
> +             (var->data.mode != ir_var_shader_in &&
> +              var->data.mode != ir_var_shader_out))
> +            continue;
> +
> +         /* Vertex shader inputs and fragment shader outputs are validated
> +          * in assign_attribute_or_color_locations, skip them.
> +          */
> +         if (sh->Stage == MESA_SHADER_FRAGMENT &&
> +             var->data.mode == ir_var_shader_out)
> +            continue;
> +
> +         if (sh->Stage == MESA_SHADER_VERTEX &&
> +             var->data.mode == ir_var_shader_in)
> +            continue;
> +
> +         if (!validate_explicit_variable_location(
> +               ctx,
> +               var->data.mode == ir_var_shader_in ? explicit_locations_in :
> +                                                    explicit_locations_out,
> +               var, prog, sh)) {
> +            return;
> +         }
> +      }
> +   }
> +}
> +
> +/**
>    * Validate that outputs from one stage match inputs of another
>    */
>   void
> diff --git a/src/compiler/glsl/link_varyings.h b/src/compiler/glsl/link_varyings.h
> index 081b04ea38..e8486208a2 100644
> --- a/src/compiler/glsl/link_varyings.h
> +++ b/src/compiler/glsl/link_varyings.h
> @@ -300,6 +300,10 @@ link_varyings(struct gl_shader_program *prog, unsigned first, unsigned last,
>                 struct gl_context *ctx, void *mem_ctx);
>   
>   void
> +validate_sso_explicit_locations(struct gl_context *ctx,
> +                                struct gl_shader_program *prog);
> +
> +void
>   cross_validate_outputs_to_inputs(struct gl_context *ctx,
>                                    struct gl_shader_program *prog,
>                                    gl_linked_shader *producer,
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 3798309678..cba14f940b 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -4938,6 +4938,12 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>         prev = i;
>      }
>   
> +   /* Validate location aliasing for SSO programs (this is done during
> +    * cross_validate_outputs_to_inputs for non-SSO programs).
> +    */
> +   if (prog->SeparateShader)
> +      validate_sso_explicit_locations(ctx, prog);

Somewhere in link shaders we already calculate the first and last shader 
stages. I think we should probably pass those to this function and then 
only run the validation on the input of the first and output of the 
last, otherwise we are re-validating things the are already covered by 
the non-sso checks.

> +
>      /* Cross-validate uniform blocks between shader stages */
>      validate_interstage_uniform_blocks(prog, prog->_LinkedShaders);
>      if (!prog->data->LinkStatus)
>