The subject line should describe the change not make an assertion. How about:
glsl: ignore all but the rightmost layout-qualifier-name On Fri, 2016-10-07 at 01:52 +0300, Andres Gomez wrote: > In a declaration, when a layout qualifier appears and holds > duplicated > layout-qualifier-name, only the last occurrence should be taken into > account. Slight change to above: When a layout contains a duplicated layout-qualifier-name in a single declaration, only the last occurrence should be taken into account. > > From page 59 (page 65 of the PDF) of the GLSL 4.40 spec: > > " More than one layout qualifier may appear in a single > declaration. Additionally, the same layout-qualifier-name can > occur multiple times within a layout qualifier or across multiple > layout qualifiers in the same declaration. When the same > layout-qualifier-name occurs multiple times, in a single > declaration, the last occurrence overrides the former > occurrence(s)." > > Consider this example: > > " #version 150 > #extension GL_ARB_shading_language_420pack: enable > #extension GL_ARB_enhanced_layouts: enable > > layout(max_vertices=2, max_vertices=3) out; > layout(max_vertices=3) out;" > > Although different values for the "max_vertices" layout-qualifier- > name > should end in a compilation failure, since only the last occurrence > is > taken into account, this small piece of code from a shader is valid. How about simply: Although different values for "max_vertices" results in a compilation error. The above code is valid because max_vertices=2 is ignored. > > Hence, when merging qualifiers in an ast_type_qualifier, we now > ignore > new appearances of a same layout-qualifier-name if the > "is_single_layout_merge" parameter is on, since the GLSL parser works > in this case from right to left. When merging qualifiers in an ast_type_qualifier, we now simply ignore new appearances of a same layout-qualifier-name if the "is_single_layout_merge" parameter is true, this works because the GLSL parser processes qualifiers from right to left. With those changes this patch is: Reviewed-by: Timothy Arceri <timothy.arc...@collabora.com> > > Signed-off-by: Andres Gomez <ago...@igalia.com> > --- > src/compiler/glsl/ast_type.cpp | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/src/compiler/glsl/ast_type.cpp > b/src/compiler/glsl/ast_type.cpp > index b586f94..504b533 100644 > --- a/src/compiler/glsl/ast_type.cpp > +++ b/src/compiler/glsl/ast_type.cpp > @@ -196,7 +196,7 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, > } > > if (q.flags.q.max_vertices) { > - if (this->max_vertices) { > + if (this->max_vertices && !is_single_layout_merge) { > this->max_vertices->merge_qualifier(q.max_vertices); > } else { > this->max_vertices = q.max_vertices; > @@ -213,7 +213,7 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, > } > > if (q.flags.q.invocations) { > - if (this->invocations) { > + if (this->invocations && !is_single_layout_merge) { > this->invocations->merge_qualifier(q.invocations); > } else { > this->invocations = q.invocations; > @@ -262,7 +262,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, > unsigned buff_idx; > if (process_qualifier_constant(state, loc, "xfb_buffer", > this->xfb_buffer, > &buff_idx)) { > - if (state->out_qualifier->out_xfb_stride[buff_idx]) { > + if (state->out_qualifier->out_xfb_stride[buff_idx] > + && !is_single_layout_merge) { > state->out_qualifier->out_xfb_stride[buff_idx]- > >merge_qualifier( > new(state) ast_layout_expression(*loc, this- > >xfb_stride)); > } else { > @@ -274,7 +275,7 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, > } > > if (q.flags.q.vertices) { > - if (this->vertices) { > + if (this->vertices && !is_single_layout_merge) { > this->vertices->merge_qualifier(q.vertices); > } else { > this->vertices = q.vertices; > @@ -312,7 +313,7 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc, > > for (int i = 0; i < 3; i++) { > if (q.flags.q.local_size & (1 << i)) { > - if (this->local_size[i]) { > + if (this->local_size[i] && !is_single_layout_merge) { > this->local_size[i]->merge_qualifier(q.local_size[i]); > } else { > this->local_size[i] = q.local_size[i]; _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev