On 09/27/2016 09:12 PM, Nicolai Hähnle wrote:
On 26.09.2016 19:23, Samuel Pitoiset wrote:This is the new layout qualifier introduced by ARB_compute_variable_group_size which allows to use a variable work group size. Signed-off-by: Samuel Pitoiset <samuel.pitoi...@gmail.com> Reviewed-by: Ian Romanick <ian.d.roman...@intel.com> --- src/compiler/glsl/ast.h | 5 +++++ src/compiler/glsl/ast_type.cpp | 6 ++++++ src/compiler/glsl/glsl_parser.yy | 13 +++++++++++++ src/compiler/glsl/glsl_parser_extras.cpp | 6 ++++++ src/compiler/glsl/glsl_parser_extras.h | 6 ++++++ 5 files changed, 36 insertions(+) diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h index 4c648d0..55f009a 100644 --- a/src/compiler/glsl/ast.h +++ b/src/compiler/glsl/ast.h @@ -553,6 +553,11 @@ struct ast_type_qualifier { */ unsigned local_size:3; + /** \name Layout qualifiers for ARB_compute_variable_group_size. */ + /** \{ */ + unsigned local_size_variable:1; + /** \} */ + /** \name Layout and memory qualifiers for ARB_shader_image_load_store. */ /** \{ */ unsigned early_fragment_tests:1; diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp index f3f6b29..3f19f1f 100644 --- a/src/compiler/glsl/ast_type.cpp +++ b/src/compiler/glsl/ast_type.cpp @@ -503,6 +503,7 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc, state->in_qualifier->flags.q.local_size == 0; valid_in_mask.flags.q.local_size = 7; + valid_in_mask.flags.q.local_size_variable = 1; break; default: _mesa_glsl_error(loc, state, @@ -580,6 +581,10 @@ ast_type_qualifier::merge_in_qualifier(YYLTYPE *loc, this->point_mode = q.point_mode; } + if (q.flags.q.local_size_variable) { + state->cs_input_local_size_variable_specified = true; + }What if previously a fixed local group size was specified? I think you need to check for this either here or in the next patch.
A GLSL compiler error will occur, I check this specific case in the next patch actually (ie. "glsl: reject compute shaders with fixed and variable local size").
+ if (create_node) { if (create_gs_ast) { node = new(mem_ctx) ast_gs_input_layout(*loc, q.prim_type); @@ -653,6 +658,7 @@ ast_type_qualifier::validate_flags(YYLTYPE *loc, bad.flags.q.prim_type ? " prim_type" : "", bad.flags.q.max_vertices ? " max_vertices" : "", bad.flags.q.local_size ? " local_size" : "", + bad.flags.q.local_size_variable ? " local_size_variable" : "", bad.flags.q.early_fragment_tests ? " early_fragment_tests" : "", bad.flags.q.explicit_image_format ? " image_format" : "", bad.flags.q.coherent ? " coherent" : "",You need to add a %s to the monster format string above. Doesn't this trigger a compiler warning?
No, I don't see a compiler warning with GCC 5.4.0. Good catch! :) I have fixed this.
Cheers, Nicolaidiff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy index 9e1fd9e..38cbd3f 100644 --- a/src/compiler/glsl/glsl_parser.yy +++ b/src/compiler/glsl/glsl_parser.yy @@ -1491,6 +1491,19 @@ layout_qualifier_id: } } + /* Layout qualifiers for ARB_compute_variable_group_size. */ + if (!$$.flags.i) { + if (match_layout_qualifier($1, "local_size_variable", state) == 0) { + $$.flags.q.local_size_variable = 1; + } + + if ($$.flags.i && !state->ARB_compute_variable_group_size_enable) { + _mesa_glsl_error(& @1, state, + "qualifier `local_size_variable` requires " + "ARB_compute_variable_group_size"); + } + } + if (!$$.flags.i) { _mesa_glsl_error(& @1, state, "unrecognized layout identifier " "`%s'", $1); diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp index eff5235..e200b7d 100644 --- a/src/compiler/glsl/glsl_parser_extras.cpp +++ b/src/compiler/glsl/glsl_parser_extras.cpp @@ -297,6 +297,8 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx, sizeof(this->atomic_counter_offsets)); this->allow_extension_directive_midshader = ctx->Const.AllowGLSLExtensionDirectiveMidShader; + + this->cs_input_local_size_variable_specified = false; } /** @@ -1676,6 +1678,7 @@ set_shader_inout_layout(struct gl_shader *shader, if (shader->Stage != MESA_SHADER_COMPUTE) { /* Should have been prevented by the parser. */ assert(!state->cs_input_local_size_specified); + assert(!state->cs_input_local_size_variable_specified); } if (shader->Stage != MESA_SHADER_FRAGMENT) { @@ -1791,6 +1794,9 @@ set_shader_inout_layout(struct gl_shader *shader, for (int i = 0; i < 3; i++) shader->info.Comp.LocalSize[i] = 0; } + + shader->info.Comp.LocalSizeVariable = + state->cs_input_local_size_variable_specified; break; case MESA_SHADER_FRAGMENT: diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h index 7528df7..127edbc 100644 --- a/src/compiler/glsl/glsl_parser_extras.h +++ b/src/compiler/glsl/glsl_parser_extras.h @@ -405,6 +405,12 @@ struct _mesa_glsl_parse_state { unsigned cs_input_local_size[3]; /** + * True if a compute shader input local variable size was specified using + * a layout directive as specified by ARB_compute_variable_group_size. + */ + bool cs_input_local_size_variable_specified; + + /** * Output layout qualifiers from GLSL 1.50 (geometry shader controls), * and GLSL 4.00 (tessellation control shader). */
-- -Samuel _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
