[Mesa-dev] [PATCH] glsl: remove unecessary flags.q.subroutine_def
Timothy Arceri
tarceri at itsqueeze.com
Tue Feb 28 23:04:46 UTC 2017
On 01/03/17 03:57, Samuel Pitoiset wrote:
>
>
> On 02/26/2017 10:19 PM, Timothy Arceri wrote:
>>
>>
>> On 25/02/17 22:15, Samuel Pitoiset wrote:
>>> This bit is definitely not necessary because subroutine_list
>>> can be used instead. This frees one more bit in the flags.q
>>> struct which is nice because arb_bindless_texture will need
>>> 4 bits for the new layout qualifiers.
>>>
>>> No piglit regressions found (including compiler tests) with
>>> "-t subroutine".
>>>
>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>> ---
>>> src/compiler/glsl/ast.h | 1 -
>>> src/compiler/glsl/ast_to_hir.cpp | 6 +++---
>>> src/compiler/glsl/ast_type.cpp | 6 ++----
>>> src/compiler/glsl/glsl_parser.yy | 1 -
>>> src/compiler/glsl/glsl_parser_extras.cpp | 2 +-
>>> 5 files changed, 6 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
>>> index 11a092e41c..d27b940744 100644
>>> --- a/src/compiler/glsl/ast.h
>>> +++ b/src/compiler/glsl/ast.h
>>> @@ -607,7 +607,6 @@ struct ast_type_qualifier {
>>> /** \name Qualifiers for GL_ARB_shader_subroutine */
>>> /** \{ */
>>> unsigned subroutine:1; /**< Is this marked 'subroutine' */
>>> - unsigned subroutine_def:1; /**< Is this marked 'subroutine'
>>> with a list of types */
>>> /** \} */
>>>
>>> /** \name Qualifiers for GL_KHR_blend_equation_advanced */
>>> diff --git a/src/compiler/glsl/ast_to_hir.cpp
>>> b/src/compiler/glsl/ast_to_hir.cpp
>>> index f033d7df97..7e99faeaed 100644
>>> --- a/src/compiler/glsl/ast_to_hir.cpp
>>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>>> @@ -3510,7 +3510,7 @@ apply_layout_qualifier_to_variable(const struct
>>> ast_type_qualifier *qual,
>>> }
>>> }
>>> } else if (qual->flags.q.explicit_index) {
>>> - if (!qual->flags.q.subroutine_def)
>>> + if (!qual->subroutine_list)
>>> _mesa_glsl_error(loc, state,
>>> "explicit index requires explicit location");
>>> } else if (qual->flags.q.explicit_component) {
>>> @@ -5568,7 +5568,7 @@ ast_function::hir(exec_list *instructions,
>>> * "Subroutine declarations cannot be prototyped. It is an error
>>> to prepend
>>> * subroutine(...) to a function declaration."
>>> */
>>> - if (this->return_type->qualifier.flags.q.subroutine_def &&
>>> !is_definition) {
>>> + if (this->return_type->qualifier.subroutine_list &&
>>> !is_definition) {
>>> YYLTYPE loc = this->get_location();
>>> _mesa_glsl_error(&loc, state,
>>> "function declaration `%s' cannot have
>>> subroutine prepended",
>>> @@ -5716,7 +5716,7 @@ ast_function::hir(exec_list *instructions,
>>> sig->replace_parameters(&hir_parameters);
>>> signature = sig;
>>>
>>> - if (this->return_type->qualifier.flags.q.subroutine_def) {
>>> + if (this->return_type->qualifier.subroutine_list) {
>>> int idx;
>>>
>>> if (this->return_type->qualifier.flags.q.explicit_index) {
>>> diff --git a/src/compiler/glsl/ast_type.cpp
>>> b/src/compiler/glsl/ast_type.cpp
>>> index 96d20c10af..5f868a81f2 100644
>>> --- a/src/compiler/glsl/ast_type.cpp
>>> +++ b/src/compiler/glsl/ast_type.cpp
>>> @@ -44,7 +44,6 @@
>>> ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state
>>> *state) const
>>> ast_type_qualifier subroutine_only;
>>> subroutine_only.flags.i = 0;
>>> subroutine_only.flags.q.subroutine = 1;
>>> - subroutine_only.flags.q.subroutine_def = 1;
>>> if (state->has_explicit_uniform_location()) {
>>> subroutine_only.flags.q.explicit_index = 1;
>>> }
>>> @@ -285,8 +284,8 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
>>> }
>>> }
>>>
>>> - if (q.flags.q.subroutine_def) {
>>> - if (this->flags.q.subroutine_def) {
>>> + if (q.subroutine_list) {
>>> + if (this->subroutine_list) {
>>> _mesa_glsl_error(loc, state,
>>> "conflicting subroutine qualifiers used");
>>> } else {
>>> @@ -772,7 +771,6 @@ ast_type_qualifier::validate_flags(YYLTYPE *loc,
>>> bad.flags.q.point_mode ? " point_mode" : "",
>>> bad.flags.q.vertices ? " vertices" : "",
>>> bad.flags.q.subroutine ? " subroutine" : "",
>>> - bad.flags.q.subroutine_def ? " subroutine_def" :
>>> "",
>>> bad.flags.q.blend_support ? " blend_support" : "",
>>> bad.flags.q.inner_coverage ? " inner_coverage" :
>>> "",
>>> bad.flags.q.post_depth_coverage ? "
>>> post_depth_coverage" : "");
>>> diff --git a/src/compiler/glsl/glsl_parser.yy
>>> b/src/compiler/glsl/glsl_parser.yy
>>> index 7777d703f8..b79fcee550 100644
>>> --- a/src/compiler/glsl/glsl_parser.yy
>>> +++ b/src/compiler/glsl/glsl_parser.yy
>>> @@ -1812,7 +1812,6 @@ subroutine_qualifier:
>>> | SUBROUTINE '(' subroutine_type_list ')'
>>> {
>>> memset(& $$, 0, sizeof($$));
>>> - $$.flags.q.subroutine_def = 1;
>>
>> You need to change this to:
>>
>> $$.flags.q.subroutine = 1;
>>
>> Otherwise we won't detect if the qualifier was added when it should have
>> been etc.
>
> Are you sure it's needed to set it up? I thought it was enough to only
> set $$.subroutine_list.
Sorry I should have been more clear. I mean the function that make use
of checking the qualifier flags for validation won't work if you don't
set something here. e.g. ast_type_qualifier::validate_flags
>
>>
>>
>>> $$.subroutine_list = $3;
>>> }
>>> ;
>>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp
>>> b/src/compiler/glsl/glsl_parser_extras.cpp
>>> index 375a99a49d..e88dd071b3 100644
>>> --- a/src/compiler/glsl/glsl_parser_extras.cpp
>>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
>>> @@ -1075,7 +1075,7 @@ _mesa_ast_type_qualifier_print(const struct
>>> ast_type_qualifier *q)
>>> if (q->flags.q.subroutine)
>>> printf("subroutine ");
>>>
>>> - if (q->flags.q.subroutine_def) {
>>> + if (q->subroutine_list) {
>>> printf("subroutine (");
>>> q->subroutine_list->print();
>>> printf(")");
>>>
More information about the mesa-dev
mailing list