[Mesa-dev] [PATCH 6/9] glsl: remove redundant record_compare check when linking globals

[Timothy Arceri]

On 15/05/17 19:27, Nicolai Hähnle wrote:
> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
> 
> Unnamed struct types are now equal across stages based on the fields they
> contain, so this additional check has become unnecessary.

I was a little confused by this change. Maybe change the above sentence to:

"Unnamed struct types are now equal across stages based only on the 
fields they contain, so overriding the type to make sure names match has 
become unnecessary."

> 
> The check was originally introduced in commit 955c93dc089f ("glsl: Match
> unnamed record types across stages.")
> ---
>   src/compiler/glsl/linker.cpp | 39 +++++++++++++++++----------------------
>   1 file changed, 17 insertions(+), 22 deletions(-)
> 
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 2e7dd2b..20465db 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -900,42 +900,37 @@ cross_validate_globals(struct gl_shader_program *prog,
>   
>         /* If a global with this name has already been seen, verify that the
>          * new instance has the same type.  In addition, if the globals have
>          * initializers, the values of the initializers must be the same.
>          */
>         ir_variable *const existing = variables->get_variable(var->name);
>         if (existing != NULL) {
>            /* Check if types match. */
>            if (var->type != existing->type) {
>               if (!validate_intrastage_arrays(prog, var, existing)) {
> -               if (var->type->is_record() && existing->type->is_record()
> -                   && existing->type->record_compare(var->type)) {
> -                   existing->type = var->type;
> -               } else {
> -                  /* If it is an unsized array in a Shader Storage Block,
> -                   * two different shaders can access to different elements.
> -                   * Because of that, they might be converted to different
> -                   * sized arrays, then check that they are compatible but
> -                   * ignore the array size.
> -                   */
> -                  if (!(var->data.mode == ir_var_shader_storage &&
> -                        var->data.from_ssbo_unsized_array &&
> -                        existing->data.mode == ir_var_shader_storage &&
> -                        existing->data.from_ssbo_unsized_array &&
> -                        var->type->gl_type == existing->type->gl_type)) {
> -                     linker_error(prog, "%s `%s' declared as type "
> -                                  "`%s' and type `%s'\n",
> -                                  mode_string(var),
> -                                  var->name, var->type->name,
> -                                  existing->type->name);
> -                     return;
> -                  }
> +               /* If it is an unsized array in a Shader Storage Block,
> +                * two different shaders can access to different elements.
> +                * Because of that, they might be converted to different
> +                * sized arrays, then check that they are compatible but
> +                * ignore the array size.
> +                */
> +               if (!(var->data.mode == ir_var_shader_storage &&
> +                     var->data.from_ssbo_unsized_array &&
> +                     existing->data.mode == ir_var_shader_storage &&
> +                     existing->data.from_ssbo_unsized_array &&
> +                     var->type->gl_type == existing->type->gl_type)) {
> +                  linker_error(prog, "%s `%s' declared as type "
> +                                 "`%s' and type `%s'\n",
> +                                 mode_string(var),
> +                                 var->name, var->type->name,
> +                                 existing->type->name);
> +                  return;
>                  }
>               }
>            }
>   
>            if (var->data.explicit_location) {
>               if (existing->data.explicit_location
>                   && (var->data.location != existing->data.location)) {
>                  linker_error(prog, "explicit locations for %s "
>                               "`%s' have differing values\n",
>                               mode_string(var), var->name);
>