On 02/24/2017 10:07 AM, Andres Gomez wrote:
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 <[email protected]>
---
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:
Okay, I will update locally before pushing.
Thanks!
Reviewed-by: Andres Gomez <[email protected]>
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
