Re: [Mesa-dev] [PATCH v5 23/70] glsl: refactor parser processing of an interface block definition
On 19/09/15 01:44, Kristian Høgsberg wrote:
> On Thu, Sep 10, 2015 at 03:35:39PM +0200, Iago Toral Quiroga wrote:
>> From: Samuel Iglesias Gonsalvez
>
> I'd be more specific in the subject and say:
>
> 'glsl: Move interface block processing to glsl_parser_extras.cpp
>
> 'Refactor' can mean many different things. Maybe even clarify in the
> commit message that there are no other changes.
>
OK, I am going to update it.
Thanks,
Sam
>> ---
>> src/glsl/ast.h | 5 ++
>> src/glsl/glsl_parser.yy | 127
>> +---
>> src/glsl/glsl_parser_extras.cpp | 122 ++
>> 3 files changed, 128 insertions(+), 126 deletions(-)
>>
>> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
>> index 335f426..cca32b3 100644
>> --- a/src/glsl/ast.h
>> +++ b/src/glsl/ast.h
>> @@ -1172,4 +1172,9 @@ extern void
>> check_builtin_array_max_size(const char *name, unsigned size,
>> YYLTYPE loc, struct _mesa_glsl_parse_state
>> *state);
>>
>> +extern void _mesa_ast_process_interface_block(YYLTYPE *locp,
>> + _mesa_glsl_parse_state *state,
>> + ast_interface_block *const
>> block,
>> + const struct
>> ast_type_qualifier q);
>> +
>> #endif /* AST_H */
>> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
>> index 42108a3..7f00929 100644
>> --- a/src/glsl/glsl_parser.yy
>> +++ b/src/glsl/glsl_parser.yy
>> @@ -2634,132 +2634,7 @@ basic_interface_block:
>>block->block_name = $2;
>>block->declarations.push_degenerate_list_at_head(& $4->link);
>>
>> - if ($1.flags.q.buffer) {
>> - if (!state->has_shader_storage_buffer_objects()) {
>> -_mesa_glsl_error(& @1, state,
>> - "#version 430 /
>> GL_ARB_shader_storage_buffer_object "
>> - "required for defining shader storage blocks");
>> - } else if (state->ARB_shader_storage_buffer_object_warn) {
>> -_mesa_glsl_warning(& @1, state,
>> - "#version 430 /
>> GL_ARB_shader_storage_buffer_object "
>> - "required for defining shader storage
>> blocks");
>> - }
>> - } else if ($1.flags.q.uniform) {
>> - if (!state->has_uniform_buffer_objects()) {
>> -_mesa_glsl_error(& @1, state,
>> - "#version 140 / GL_ARB_uniform_buffer_object "
>> - "required for defining uniform blocks");
>> - } else if (state->ARB_uniform_buffer_object_warn) {
>> -_mesa_glsl_warning(& @1, state,
>> - "#version 140 / GL_ARB_uniform_buffer_object
>> "
>> - "required for defining uniform blocks");
>> - }
>> - } else {
>> - if (state->es_shader || state->language_version < 150) {
>> -_mesa_glsl_error(& @1, state,
>> - "#version 150 required for using "
>> - "interface blocks");
>> - }
>> - }
>> -
>> - /* From the GLSL 1.50.11 spec, section 4.3.7 ("Interface Blocks"):
>> - * "It is illegal to have an input block in a vertex shader
>> - * or an output block in a fragment shader"
>> - */
>> - if ((state->stage == MESA_SHADER_VERTEX) && $1.flags.q.in) {
>> - _mesa_glsl_error(& @1, state,
>> - "`in' interface block is not allowed for "
>> - "a vertex shader");
>> - } else if ((state->stage == MESA_SHADER_FRAGMENT) && $1.flags.q.out) {
>> - _mesa_glsl_error(& @1, state,
>> - "`out' interface block is not allowed for "
>> - "a fragment shader");
>> - }
>> -
>> - /* Since block arrays require names, and both features are added in
>> - * the same language versions, we don't have to explicitly
>> - * version-check both things.
>> - */
>> - if (block->instance_name != NULL) {
>> - state->check_version(150, 300, & @1, "interface blocks with "
>> - "an instance name are not allowed");
>> - }
>> -
>> - uint64_t interface_type_mask;
>> - struct ast_type_qualifier temp_type_qualifier;
>> -
>> - /* Get a bitmask containing only the in/out/uniform/buffer
>> - * flags, allowing us to ignore other irrelevant flags like
>> - * interpolation qualifiers.
>> - */
>> - temp_type_qualifier.flags.i = 0;
>> - temp_type_qualifier.flags.q.uniform = true;
>> - temp_type_qualifier.flags.q.buffer = true;
>> - temp_type_qualifier.flags.q.in = true;
>> - temp_type_qualifier.flags.q.out = true;
>> - interface_type_mask = temp_type_qualifier.flags.i;
>> -
>> - /* Get t
Re: [Mesa-dev] [PATCH v5 23/70] glsl: refactor parser processing of an interface block definition
On Thu, Sep 10, 2015 at 03:35:39PM +0200, Iago Toral Quiroga wrote:
> From: Samuel Iglesias Gonsalvez
I'd be more specific in the subject and say:
'glsl: Move interface block processing to glsl_parser_extras.cpp
'Refactor' can mean many different things. Maybe even clarify in the
commit message that there are no other changes.
> ---
> src/glsl/ast.h | 5 ++
> src/glsl/glsl_parser.yy | 127
> +---
> src/glsl/glsl_parser_extras.cpp | 122 ++
> 3 files changed, 128 insertions(+), 126 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 335f426..cca32b3 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -1172,4 +1172,9 @@ extern void
> check_builtin_array_max_size(const char *name, unsigned size,
> YYLTYPE loc, struct _mesa_glsl_parse_state
> *state);
>
> +extern void _mesa_ast_process_interface_block(YYLTYPE *locp,
> + _mesa_glsl_parse_state *state,
> + ast_interface_block *const
> block,
> + const struct
> ast_type_qualifier q);
> +
> #endif /* AST_H */
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 42108a3..7f00929 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -2634,132 +2634,7 @@ basic_interface_block:
>block->block_name = $2;
>block->declarations.push_degenerate_list_at_head(& $4->link);
>
> - if ($1.flags.q.buffer) {
> - if (!state->has_shader_storage_buffer_objects()) {
> -_mesa_glsl_error(& @1, state,
> - "#version 430 /
> GL_ARB_shader_storage_buffer_object "
> - "required for defining shader storage blocks");
> - } else if (state->ARB_shader_storage_buffer_object_warn) {
> -_mesa_glsl_warning(& @1, state,
> - "#version 430 /
> GL_ARB_shader_storage_buffer_object "
> - "required for defining shader storage
> blocks");
> - }
> - } else if ($1.flags.q.uniform) {
> - if (!state->has_uniform_buffer_objects()) {
> -_mesa_glsl_error(& @1, state,
> - "#version 140 / GL_ARB_uniform_buffer_object "
> - "required for defining uniform blocks");
> - } else if (state->ARB_uniform_buffer_object_warn) {
> -_mesa_glsl_warning(& @1, state,
> - "#version 140 / GL_ARB_uniform_buffer_object "
> - "required for defining uniform blocks");
> - }
> - } else {
> - if (state->es_shader || state->language_version < 150) {
> -_mesa_glsl_error(& @1, state,
> - "#version 150 required for using "
> - "interface blocks");
> - }
> - }
> -
> - /* From the GLSL 1.50.11 spec, section 4.3.7 ("Interface Blocks"):
> - * "It is illegal to have an input block in a vertex shader
> - * or an output block in a fragment shader"
> - */
> - if ((state->stage == MESA_SHADER_VERTEX) && $1.flags.q.in) {
> - _mesa_glsl_error(& @1, state,
> - "`in' interface block is not allowed for "
> - "a vertex shader");
> - } else if ((state->stage == MESA_SHADER_FRAGMENT) && $1.flags.q.out) {
> - _mesa_glsl_error(& @1, state,
> - "`out' interface block is not allowed for "
> - "a fragment shader");
> - }
> -
> - /* Since block arrays require names, and both features are added in
> - * the same language versions, we don't have to explicitly
> - * version-check both things.
> - */
> - if (block->instance_name != NULL) {
> - state->check_version(150, 300, & @1, "interface blocks with "
> - "an instance name are not allowed");
> - }
> -
> - uint64_t interface_type_mask;
> - struct ast_type_qualifier temp_type_qualifier;
> -
> - /* Get a bitmask containing only the in/out/uniform/buffer
> - * flags, allowing us to ignore other irrelevant flags like
> - * interpolation qualifiers.
> - */
> - temp_type_qualifier.flags.i = 0;
> - temp_type_qualifier.flags.q.uniform = true;
> - temp_type_qualifier.flags.q.buffer = true;
> - temp_type_qualifier.flags.q.in = true;
> - temp_type_qualifier.flags.q.out = true;
> - interface_type_mask = temp_type_qualifier.flags.i;
> -
> - /* Get the block's interface qualifier. The interface_qualifier
> - * production rule guarantees that only one bit will be set (and
> - * it will be in/out/uniform).
> - */
> - uint64_t block_interf
[Mesa-dev] [PATCH v5 23/70] glsl: refactor parser processing of an interface block definition
From: Samuel Iglesias Gonsalvez
---
src/glsl/ast.h | 5 ++
src/glsl/glsl_parser.yy | 127 +---
src/glsl/glsl_parser_extras.cpp | 122 ++
3 files changed, 128 insertions(+), 126 deletions(-)
diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 335f426..cca32b3 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -1172,4 +1172,9 @@ extern void
check_builtin_array_max_size(const char *name, unsigned size,
YYLTYPE loc, struct _mesa_glsl_parse_state
*state);
+extern void _mesa_ast_process_interface_block(YYLTYPE *locp,
+ _mesa_glsl_parse_state *state,
+ ast_interface_block *const block,
+ const struct ast_type_qualifier
q);
+
#endif /* AST_H */
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 42108a3..7f00929 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -2634,132 +2634,7 @@ basic_interface_block:
block->block_name = $2;
block->declarations.push_degenerate_list_at_head(& $4->link);
- if ($1.flags.q.buffer) {
- if (!state->has_shader_storage_buffer_objects()) {
-_mesa_glsl_error(& @1, state,
- "#version 430 /
GL_ARB_shader_storage_buffer_object "
- "required for defining shader storage blocks");
- } else if (state->ARB_shader_storage_buffer_object_warn) {
-_mesa_glsl_warning(& @1, state,
- "#version 430 /
GL_ARB_shader_storage_buffer_object "
- "required for defining shader storage blocks");
- }
- } else if ($1.flags.q.uniform) {
- if (!state->has_uniform_buffer_objects()) {
-_mesa_glsl_error(& @1, state,
- "#version 140 / GL_ARB_uniform_buffer_object "
- "required for defining uniform blocks");
- } else if (state->ARB_uniform_buffer_object_warn) {
-_mesa_glsl_warning(& @1, state,
- "#version 140 / GL_ARB_uniform_buffer_object "
- "required for defining uniform blocks");
- }
- } else {
- if (state->es_shader || state->language_version < 150) {
-_mesa_glsl_error(& @1, state,
- "#version 150 required for using "
- "interface blocks");
- }
- }
-
- /* From the GLSL 1.50.11 spec, section 4.3.7 ("Interface Blocks"):
- * "It is illegal to have an input block in a vertex shader
- * or an output block in a fragment shader"
- */
- if ((state->stage == MESA_SHADER_VERTEX) && $1.flags.q.in) {
- _mesa_glsl_error(& @1, state,
- "`in' interface block is not allowed for "
- "a vertex shader");
- } else if ((state->stage == MESA_SHADER_FRAGMENT) && $1.flags.q.out) {
- _mesa_glsl_error(& @1, state,
- "`out' interface block is not allowed for "
- "a fragment shader");
- }
-
- /* Since block arrays require names, and both features are added in
- * the same language versions, we don't have to explicitly
- * version-check both things.
- */
- if (block->instance_name != NULL) {
- state->check_version(150, 300, & @1, "interface blocks with "
- "an instance name are not allowed");
- }
-
- uint64_t interface_type_mask;
- struct ast_type_qualifier temp_type_qualifier;
-
- /* Get a bitmask containing only the in/out/uniform/buffer
- * flags, allowing us to ignore other irrelevant flags like
- * interpolation qualifiers.
- */
- temp_type_qualifier.flags.i = 0;
- temp_type_qualifier.flags.q.uniform = true;
- temp_type_qualifier.flags.q.buffer = true;
- temp_type_qualifier.flags.q.in = true;
- temp_type_qualifier.flags.q.out = true;
- interface_type_mask = temp_type_qualifier.flags.i;
-
- /* Get the block's interface qualifier. The interface_qualifier
- * production rule guarantees that only one bit will be set (and
- * it will be in/out/uniform).
- */
- uint64_t block_interface_qualifier = $1.flags.i;
-
- block->layout.flags.i |= block_interface_qualifier;
-
- if (state->stage == MESA_SHADER_GEOMETRY &&
- state->has_explicit_attrib_stream()) {
- /* Assign global layout's stream value. */
- block->layout.flags.q.stream = 1;
- block->layout.flags.q.explicit_stream = 0;
- block->layout.stream = state->out_qualifier->stream;
- }
-
- foreach_list_typed (ast_declarator_list, member, link,
&block->declarations) {
- as
