Thanks for the review Nicolai! However, I sent a v2 yesterday that handles some cases better:
https://lists.freedesktop.org/archives/mesa-dev/2017-October/173062.htm l Specifically, this v2 computes the location slot for patch variables correctly and fixes a couple of regressions in piglit that I had missed when I sent the v1. Does your review stand for the v2? Iago On Tue, 2017-10-17 at 21:22 +0200, Nicolai Hähnle wrote: > On 16.10.2017 14:06, Iago Toral Quiroga wrote: > > We only need to add a check to validate output locations here. For > > inputs with invalid locations we will fail to link when we can't > > find a matching output in the same (invalid) location. > > > > Fixes: > > KHR-GL45.enhanced_layouts.varying_location_limit > > Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com> > > > > --- > > src/compiler/glsl/link_varyings.cpp | 12 +++++++++++- > > src/compiler/glsl/link_varyings.h | 3 ++- > > src/compiler/glsl/linker.cpp | 2 +- > > 3 files changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/src/compiler/glsl/link_varyings.cpp > > b/src/compiler/glsl/link_varyings.cpp > > index 29842ecacd..bdb70edf47 100644 > > --- a/src/compiler/glsl/link_varyings.cpp > > +++ b/src/compiler/glsl/link_varyings.cpp > > @@ -381,7 +381,8 @@ cross_validate_front_and_back_color(struct > > gl_shader_program *prog, > > * Validate that outputs from one stage match inputs of another > > */ > > void > > -cross_validate_outputs_to_inputs(struct gl_shader_program *prog, > > +cross_validate_outputs_to_inputs(struct gl_context *ctx, > > + struct gl_shader_program *prog, > > gl_linked_shader *producer, > > gl_linked_shader *consumer) > > { > > @@ -410,6 +411,15 @@ cross_validate_outputs_to_inputs(struct > > gl_shader_program *prog, > > unsigned slot_limit = idx + num_elements; > > unsigned last_comp; > > > > + unsigned slot_max = > > + ctx->Const.Program[producer- > > >Stage].MaxOutputComponents / 4; > > + if (slot_limit > slot_max) { > > + linker_error(prog, > > + "Invalid location %u in %s shader\n", > > + idx, > > _mesa_shader_stage_to_string(producer->Stage)); > > + return; > > + } > > + > > if (type->without_array()->is_record()) { > > /* The component qualifier can't be used on structs > > so just treat > > * all component slots as used. > > diff --git a/src/compiler/glsl/link_varyings.h > > b/src/compiler/glsl/link_varyings.h > > index 4e1f6d2e42..081b04ea38 100644 > > --- a/src/compiler/glsl/link_varyings.h > > +++ b/src/compiler/glsl/link_varyings.h > > @@ -300,7 +300,8 @@ link_varyings(struct gl_shader_program *prog, > > unsigned first, unsigned last, > > struct gl_context *ctx, void *mem_ctx); > > > > void > > -cross_validate_outputs_to_inputs(struct gl_shader_program *prog, > > +cross_validate_outputs_to_inputs(struct gl_context *ctx, > > + struct gl_shader_program *prog, > > gl_linked_shader *producer, > > gl_linked_shader *consumer); > > > > diff --git a/src/compiler/glsl/linker.cpp > > b/src/compiler/glsl/linker.cpp > > index 03eb05bf63..3798309678 100644 > > --- a/src/compiler/glsl/linker.cpp > > +++ b/src/compiler/glsl/linker.cpp > > @@ -4929,7 +4929,7 @@ link_shaders(struct gl_context *ctx, struct > > gl_shader_program *prog) > > if (!prog->data->LinkStatus) > > goto done; > > > > - cross_validate_outputs_to_inputs(prog, > > + cross_validate_outputs_to_inputs(ctx, prog, > > prog- > > >_LinkedShaders[prev], > > prog->_LinkedShaders[i]); > > if (!prog->data->LinkStatus) > > > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
