[Mesa-dev] [PATCH V2 11/12] glsl: add subroutine index qualifier support

[Emil Velikov]

On 8 November 2015 at 22:34, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> From: Timothy Arceri <timothy.arceri at collabora.com>
>
> ARB_explicit_uniform_location allows the index for subroutine functions
> to be explicitly set in the shader.
>
> This patch reduces the restriction on the index qualifier in
> validate_layout_qualifiers() to allow it to be applied to subroutines
> and adds the new subroutine qualifier validation to ast_function::hir().
>
> ast_fully_specified_type::has_qualifiers() is updated to allow the
> index qualifier on subroutine functions when explicit uniform loctions
typo "locations"

> is available.
>
> A new check is added to ast_type_qualifier::merge_qualifier() to stop
> multiple function qualifiers from being defied, before this patch this
> would cause a segfault.
>
> Finally a new variable is added to ir_function_signature to store the
> index. This value is validated and the non explicit values assigned in
> link_assign_subroutine_types().
> ---
>  src/glsl/ast.h                 |  2 +-
>  src/glsl/ast_to_hir.cpp        | 68 ++++++++++++++++++++++++++++++------------
>  src/glsl/ast_type.cpp          | 14 ++++++++-
>  src/glsl/ir.cpp                |  1 +
>  src/glsl/ir.h                  |  2 ++
>  src/glsl/ir_clone.cpp          |  1 +
>  src/glsl/linker.cpp            | 33 ++++++++++++++++++++
>  src/mesa/main/mtypes.h         |  1 +
>  src/mesa/main/shader_query.cpp |  7 +++++
>  9 files changed, 108 insertions(+), 21 deletions(-)
>

> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp

> @@ -4983,6 +4992,27 @@ ast_function::hir(exec_list *instructions,
>     if (this->return_type->qualifier.flags.q.subroutine_def) {
>        int idx;
>
> +      if (this->return_type->qualifier.flags.q.explicit_index) {
> +         unsigned qual_index;
> +         if (process_qualifier_constant(state, &loc, "index",
> +                                        this->return_type->qualifier.index,
> +                                        &qual_index, 0)) {
> +            if (!state->has_explicit_uniform_location()) {
> +               _mesa_glsl_error(&loc, state, "subroutine index requires "
> +                                "GL_ARB_explicit_uniform_location or "
> +                                "GLSL 4.30");
> +            } else if (qual_index > (MAX_SUBROUTINES - 1)) {
Afaict the more common approach is to use foo >= maxfoo

> +               _mesa_glsl_error(&loc, state,
> +                                "invalid subroutine index (%d) index must "
> +                                "be a number between 0 and "
> +                                "GL_MAX_SUBROUTINES - 1 (%d)", qual_index,
> +                                MAX_SUBROUTINES - 1);
Nitpick: flesh the above into a separate validate() function

-Emil