[Mesa-dev] [PATCH] glsl: remove unecessary flags.q.subroutine_def

[Samuel Pitoiset]

Thanks for reporting this. I will investigate.

On 03/01/2017 08:36 PM, Mark Janes wrote:
> This patch breaks gl45-cts.shader_subroutine.static_subroutine_call.
>
> It also breaks
> piglit.spec.arb_shader_subroutine.compiler.direct-call_vert:
>
> Failed to compile vertex shader piglit/tests/spec/arb_shader_subroutine/compiler/direct-call.vert: 0:26(2): error: subroutine name cannot be a constructor `impl'
>
> Samuel Pitoiset <samuel.pitoiset at gmail.com> writes:
>
>> 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;
>>        $$.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(")");
>> --
>> 2.11.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev