[Mesa-dev] [PATCH 1/2] glsl: add a mechanism to allow layout qualifiers on function params

[Marek Olšák]

Uh.

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Wed, Aug 29, 2018 at 1:48 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> The spec is quite clear this is not allowed:
>
>     From Section 4.4. (Layout Qualifiers) of the GLSL 4.60 spec:
>
>        "Layout qualifiers can appear in several forms of declaration.
>        They can appear as part of an interface block definition or
>        block member, as shown in the grammar in the previous section.
>        They can also appear with just an interface-qualifier to establish
>        layouts of other declarations made with that qualifier:
>
>           layout-qualifier interface-qualifier ;
>
>        Or, they can appear with an individual variable declared with
>        an interface qualifier:
>
>           layout-qualifier interface-qualifier declaration ;"
>
>     From Section 4.10 (Memory Qualifiers) of the GLSL 4.60 spec:
>
>        "Layout qualifiers cannot be used on formal function parameters,
>        and layout qualification is not included in parameter matching."
>
> However on the Nvidia binary driver they actually fail to compile
> if image function params don't have a layout qualifier. This results
> in applications such as No Mans Sky using layout qualifiers on params.
>
> I've submitted a CTS test to expose this problem in the Nvidia driver
> but until that is resolved this patch will help Mesa drivers work
> around the issue.
> ---
>  src/compiler/glsl/glsl_parser.yy         | 17 +++++++++++++++++
>  src/compiler/glsl/glsl_parser_extras.cpp |  2 ++
>  src/compiler/glsl/glsl_parser_extras.h   |  1 +
>  src/mesa/main/mtypes.h                   |  5 +++++
>  4 files changed, 25 insertions(+)
>
> diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
> index bc2571b6844..fd1592beca0 100644
> --- a/src/compiler/glsl/glsl_parser.yy
> +++ b/src/compiler/glsl/glsl_parser.yy
> @@ -897,6 +897,23 @@ parameter_declarator:
>        $$->identifier = $2;
>        state->symbols->add_variable(new(state) ir_variable(NULL, $2, ir_var_auto));
>     }
> +   | layout_qualifier type_specifier any_identifier
> +   {
> +      if (state->allow_layout_qualifier_on_function_parameter) {
> +         void *ctx = state->linalloc;
> +         $$ = new(ctx) ast_parameter_declarator();
> +         $$->set_location_range(@2, @3);
> +         $$->type = new(ctx) ast_fully_specified_type();
> +         $$->type->set_location(@2);
> +         $$->type->specifier = $2;
> +         $$->identifier = $3;
> +         state->symbols->add_variable(new(state) ir_variable(NULL, $3, ir_var_auto));
> +      } else {
> +         _mesa_glsl_error(&@1, state,
> +                          "is is not allowed on function parameter");
> +         YYERROR;
> +      }
> +   }
>     | type_specifier any_identifier array_specifier
>     {
>        void *ctx = state->linalloc;
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index 0a7d0d78b14..efd1a013dbd 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -311,6 +311,8 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
>        ctx->Const.AllowGLSLExtensionDirectiveMidShader;
>     this->allow_builtin_variable_redeclaration =
>        ctx->Const.AllowGLSLBuiltinVariableRedeclaration;
> +   this->allow_layout_qualifier_on_function_parameter =
> +      ctx->Const.AllowLayoutQualifiersOnFunctionParameters;
>
>     this->cs_input_local_size_variable_specified = false;
>
> diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h
> index 2c8353214aa..69aa6cf9cf3 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -866,6 +866,7 @@ struct _mesa_glsl_parse_state {
>
>     bool allow_extension_directive_midshader;
>     bool allow_builtin_variable_redeclaration;
> +   bool allow_layout_qualifier_on_function_parameter;
>
>     /**
>      * Known subroutine type declarations.
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 9d058cef6d9..1f640b063c0 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3764,6 +3764,11 @@ struct gl_constants
>      */
>     GLboolean AllowHigherCompatVersion;
>
> +   /**
> +    * Allow layout qualifiers on function parameters.
> +    */
> +   GLboolean AllowLayoutQualifiersOnFunctionParameters;
> +
>     /**
>      * Force computing the absolute value for sqrt() and inversesqrt() to follow
>      * D3D9 when apps rely on this behaviour.
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev