[Mesa-dev] [PATCH V3 11/28] glsl: cross validate varyings with a component qualifier

[Anuj Phogat]

On Thu, Jan 7, 2016 at 3:15 PM, Timothy Arceri
<timothy.arceri at collabora.com> wrote:
> This change checks for component overlap, including handling overlap of
> locations and components by doubles. Previously there was no validation
> for assigning explicit locations to a location used by the second half
> of a double.
>
> V3: simplify handling of doubles and fix double component aliasing
> detection
>
> V2: fix component matching for matricies
>
> Cc: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/glsl/link_varyings.cpp | 63 ++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 52 insertions(+), 11 deletions(-)
>
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 6a9ee94..03c131a 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -222,7 +222,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>                                  gl_shader *producer, gl_shader *consumer)
>  {
>     glsl_symbol_table parameters;
> -   ir_variable *explicit_locations[MAX_VARYING] = { NULL, };
> +   ir_variable *explicit_locations[MAX_VARYING][4] = { {NULL, NULL} };
>
>     /* Find all shader outputs in the "producer" stage.
>      */
> @@ -243,18 +243,59 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>           unsigned num_elements = type->count_attribute_slots(false);
>           unsigned idx = var->data.location - VARYING_SLOT_VAR0;
>           unsigned slot_limit = idx + num_elements;
> +         unsigned last_comp;
> +
> +         if (var->type->without_array()->is_record()) {
> +            /* The componet qualifier can't be used on structs so just treat
> +             * all component slots as used.
> +             */
> +            last_comp = 4;
> +         } else {
> +            unsigned dmul = var->type->is_double() ? 2 : 1;
> +            last_comp = var->data.location_frac +
> +               var->type->without_array()->vector_elements * dmul;
> +         }
>
>           while(idx < slot_limit) {
> -            if (explicit_locations[idx] != NULL) {
> -               linker_error(prog,
> -                         "%s shader has multiple outputs explicitly "
> -                         "assigned to location %d\n",
> -                         _mesa_shader_stage_to_string(producer->Stage),
> -                         idx);
> -               return;
> -            }
> +            for (unsigned i = var->data.location_frac; i < last_comp; i++) {
> +               if (explicit_locations[idx][i] != NULL) {
> +                  linker_error(prog,
> +                               "%s shader has multiple outputs explicitly "
> +                               "assigned to location %d and component %d\n",
> +                               _mesa_shader_stage_to_string(producer->Stage),
> +                               idx, var->data.location_frac);
> +                  return;
> +               }
>
> -            explicit_locations[idx] = var;
> +               /* Make sure all component at this location have the same type.
> +                */
> +               for (unsigned j = 0; j < 4; j++) {
> +                  if (explicit_locations[idx][j] &&
> +                      (explicit_locations[idx][j]->type->without_array()
> +                       ->base_type != var->type->without_array()->base_type)) {
> +                     linker_error(prog,
> +                                  "Varyings sharing the same location must "
> +                                  "have the same underlying numerical type. "
> +                                  "Location %u component %u\n", idx,
> +                                  var->data.location_frac);
> +                     return;
> +                  }
> +               }
> +
> +               explicit_locations[idx][i] = var;
> +
> +               /* We need to do some special handling for doubles as dvec3 and
> +                * dvec4 consume two consecutive locations. We don't need to
> +                * worry about components beginning at anything other than 0 as
> +                * the spec does not allow this for dvec3 and dvec4.
> +                */
> +               if (i == 3 && last_comp > 4) {
> +                  last_comp = last_comp - 4;
> +                  /* Bump location index and reset the component index */
> +                  idx++;
> +                  i = 0;
> +               }
> +            }
>              idx++;
>           }
>        }
> @@ -311,7 +352,7 @@ cross_validate_outputs_to_inputs(struct gl_shader_program *prog,
>              unsigned slot_limit = idx + num_elements;
>
>              while(idx < slot_limit) {
> -               output = explicit_locations[idx];
> +               output = explicit_locations[idx][input->data.location_frac];
>
>                 if (output == NULL ||
>                     input->data.location != output->data.location) {
> --
> 2.4.3
>

Thanks for the fix.
Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>