[Mesa-dev] [PATCH v3 04/14] glsl: process local_size_variable input qualifier
Samuel Pitoiset
samuel.pitoiset at gmail.com
Wed Oct 5 14:06:56 UTC 2016
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.pitoiset at gmail.com>
>> Reviewed-by: Ian Romanick <ian.d.romanick at 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,
> Nicolai
>
>
>> diff --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
More information about the mesa-dev
mailing list