On Thu, 2017-02-23 at 18:07 +0100, Samuel Pitoiset wrote: > The main idea behind this is to free some bits in the flags.q > struct because currently all 64-bits are used and we can't > add more layout qualifiers without reaching a static assert. > > In order to do that (mainly for ARB_bindless_texture), use an > enumeration for the AMD_conservative_depth layout qualifiers > because it's forbidden to declare more than one depth qualifier > for gl_FragDepth. > > Note that ast_type_qualifier::merge_qualifier() will prevent > using duplicate layout qualifiers by returning a compile-time > error. > > No piglit regressions found (including compiler tests) with > RX480 on RadeonSI. > > Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com> > --- > src/compiler/glsl/ast.h | 16 ++++++++++++---- > src/compiler/glsl/ast_to_hir.cpp | 21 ++++++--------------- > src/compiler/glsl/ast_type.cpp | 12 +++--------- > src/compiler/glsl/glsl_parser.yy | 12 ++++++++---- > 4 files changed, 29 insertions(+), 32 deletions(-) > > diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h > index 3dff164232..11a092e41c 100644 > --- a/src/compiler/glsl/ast.h > +++ b/src/compiler/glsl/ast.h > @@ -463,6 +463,14 @@ enum { > ast_precision_low > }; > > +enum { > + ast_depth_none = 0, /**< Absence of depth qualifier. */ > + ast_depth_any, > + ast_depth_greater, > + ast_depth_less, > + ast_depth_unchanged > +}; > + > struct ast_type_qualifier { > DECLARE_RALLOC_CXX_OPERATORS(ast_type_qualifier); > > @@ -528,10 +536,7 @@ struct ast_type_qualifier { > > /** \name Layout qualifiers for GL_AMD_conservative_depth */ > /** \{ */ > - unsigned depth_any:1; > - unsigned depth_greater:1; > - unsigned depth_less:1; > - unsigned depth_unchanged:1; > + unsigned depth_type:1; > /** \} */ > > /** \name Layout qualifiers for GL_ARB_uniform_buffer_object */ > @@ -630,6 +635,9 @@ struct ast_type_qualifier { > /** Precision of the type (highp/medium/lowp). */ > unsigned precision:2; > > + /** Type of layout qualifiers for GL_AMD_conservative_depth. */ > + unsigned depth_type:3; > + > /** > * Alignment specified via GL_ARB_enhanced_layouts "align" layout > qualifier > */ > diff --git a/src/compiler/glsl/ast_to_hir.cpp > b/src/compiler/glsl/ast_to_hir.cpp > index 88febe7ff3..ad1fd9150a 100644 > --- a/src/compiler/glsl/ast_to_hir.cpp > +++ b/src/compiler/glsl/ast_to_hir.cpp > @@ -3629,11 +3629,7 @@ apply_layout_qualifier_to_variable(const struct > ast_type_qualifier *qual, > /* Layout qualifiers for gl_FragDepth, which are enabled by extension > * AMD_conservative_depth. > */ > - int depth_layout_count = qual->flags.q.depth_any > - + qual->flags.q.depth_greater > - + qual->flags.q.depth_less > - + qual->flags.q.depth_unchanged; > - if (depth_layout_count > 0 > + if (qual->flags.q.depth_type > && !state->is_version(420, 0) > && !state->AMD_conservative_depth_enable > && !state->ARB_conservative_depth_enable) { > @@ -3641,24 +3637,19 @@ apply_layout_qualifier_to_variable(const struct > ast_type_qualifier *qual, > "extension GL_AMD_conservative_depth or " > "GL_ARB_conservative_depth must be enabled " > "to use depth layout qualifiers"); > - } else if (depth_layout_count > 0 > + } else if (qual->flags.q.depth_type > && strcmp(var->name, "gl_FragDepth") != 0) { > _mesa_glsl_error(loc, state, > "depth layout qualifiers can be applied only to " > "gl_FragDepth"); > - } else if (depth_layout_count > 1 > - && strcmp(var->name, "gl_FragDepth") == 0) { > - _mesa_glsl_error(loc, state, > - "at most one depth layout qualifier can be applied to > " > - "gl_FragDepth"); > } > - if (qual->flags.q.depth_any) > + if (qual->depth_type == ast_depth_any) > var->data.depth_layout = ir_depth_layout_any; > - else if (qual->flags.q.depth_greater) > + else if (qual->depth_type == ast_depth_greater) > var->data.depth_layout = ir_depth_layout_greater; > - else if (qual->flags.q.depth_less) > + else if (qual->depth_type == ast_depth_less) > var->data.depth_layout = ir_depth_layout_less; > - else if (qual->flags.q.depth_unchanged) > + else if (qual->depth_type == ast_depth_unchanged) > var->data.depth_layout = ir_depth_layout_unchanged; > else > var->data.depth_layout = ir_depth_layout_none;
Maybe replace this with a ? switch(qual->depth_type) { case ast_depth_any: ... } Other than that nitpick, this is: Reviewed-by: Andres Gomez <ago...@igalia.com> -- Br, Andres _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev