[Mesa-dev] [PATCH v3 13/14] glsl: add subroutine index qualifier support
Tapani Pälli
tapani.palli at intel.com
Fri Nov 20 02:04:52 PST 2015
Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
On 11/14/2015 03:42 PM, Timothy Arceri 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 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 | 34 ++++++++++++++++++++++++++++++++--
> 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, 91 insertions(+), 4 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index bfeab6b..1e4a998 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -772,7 +772,7 @@ public:
> class ast_fully_specified_type : public ast_node {
> public:
> virtual void print(void) const;
> - bool has_qualifiers() const;
> + bool has_qualifiers(_mesa_glsl_parse_state *state) const;
>
> ast_fully_specified_type() : qualifier(), specifier(NULL)
> {
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 6a3ec44..6c56829 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -2848,6 +2848,13 @@ apply_explicit_location(const struct ast_type_qualifier *qual,
> break;
> }
>
> + /* Check if index was set for the uniform instead of the function */
> + if (qual->flags.q.explicit_index && qual->flags.q.subroutine) {
> + _mesa_glsl_error(loc, state, "an index qualifier can only be "
> + "used with subroutine functions");
> + return;
> + }
> +
> unsigned qual_index;
> if (qual->flags.q.explicit_index &&
> process_qualifier_constant(state, loc, "index", qual->index,
> @@ -3067,7 +3074,9 @@ apply_layout_qualifier_to_variable(const struct ast_type_qualifier *qual,
> if (qual->flags.q.explicit_location) {
> apply_explicit_location(qual, var, state, loc);
> } else if (qual->flags.q.explicit_index) {
> - _mesa_glsl_error(loc, state, "explicit index requires explicit location");
> + if (!qual->flags.q.subroutine_def)
> + _mesa_glsl_error(loc, state,
> + "explicit index requires explicit location");
> }
>
> if (qual->flags.q.explicit_binding) {
> @@ -5075,7 +5084,7 @@ ast_function::hir(exec_list *instructions,
> /* From page 56 (page 62 of the PDF) of the GLSL 1.30 spec:
> * "No qualifier is allowed on the return type of a function."
> */
> - if (this->return_type->has_qualifiers()) {
> + if (this->return_type->has_qualifiers(state)) {
> YYLTYPE loc = this->get_location();
> _mesa_glsl_error(& loc, state,
> "function `%s' return type has qualifiers", name);
> @@ -5207,6 +5216,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)) {
> + 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) {
> + _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);
> + } else {
> + f->subroutine_index = qual_index;
> + }
> + }
> + }
> +
> f->num_subroutine_types = this->return_type->qualifier.subroutine_list->declarations.length();
> f->subroutine_types = ralloc_array(state, const struct glsl_type *,
> f->num_subroutine_types);
> diff --git a/src/glsl/ast_type.cpp b/src/glsl/ast_type.cpp
> index 1e89a9e..03ed4dc 100644
> --- a/src/glsl/ast_type.cpp
> +++ b/src/glsl/ast_type.cpp
> @@ -38,13 +38,16 @@ ast_type_specifier::print(void) const
> }
>
> bool
> -ast_fully_specified_type::has_qualifiers() const
> +ast_fully_specified_type::has_qualifiers(_mesa_glsl_parse_state *state) const
> {
> /* 'subroutine' isnt a real qualifier. */
> 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;
> + }
> return (this->qualifier.flags.i & ~subroutine_only.flags.i) != 0;
> }
>
> @@ -176,6 +179,15 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
> }
> }
>
> + if (q.flags.q.subroutine_def) {
> + if (this->flags.q.subroutine_def) {
> + _mesa_glsl_error(loc, state,
> + "conflicting subroutine qualifiers used");
> + } else {
> + this->subroutine_list = q.subroutine_list;
> + }
> + }
> +
> if (q.flags.q.invocations) {
> if (this->invocations) {
> this->invocations->merge_qualifier(q.invocations);
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 8933b23..edd2818 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -1842,6 +1842,7 @@ ir_function_signature::replace_parameters(exec_list *new_params)
> ir_function::ir_function(const char *name)
> : ir_instruction(ir_type_function)
> {
> + this->subroutine_index = -1;
> this->name = ralloc_strdup(this, name);
> }
>
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index d59dee1..a0dfd41 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -1171,6 +1171,8 @@ public:
> */
> int num_subroutine_types;
> const struct glsl_type **subroutine_types;
> +
> + int subroutine_index;
> };
>
> inline const char *ir_function_signature::function_name() const
> diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
> index a2cd672..ea8fbd2 100644
> --- a/src/glsl/ir_clone.cpp
> +++ b/src/glsl/ir_clone.cpp
> @@ -269,6 +269,7 @@ ir_function::clone(void *mem_ctx, struct hash_table *ht) const
> ir_function *copy = new(mem_ctx) ir_function(this->name);
>
> copy->is_subroutine = this->is_subroutine;
> + copy->subroutine_index = this->subroutine_index;
> copy->num_subroutine_types = this->num_subroutine_types;
> copy->subroutine_types = ralloc_array(mem_ctx, const struct glsl_type *, copy->num_subroutine_types);
> for (int i = 0; i < copy->num_subroutine_types; i++)
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index db00f8f..331d9a2 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -3864,10 +3864,43 @@ link_assign_subroutine_types(struct gl_shader_program *prog)
> sh->SubroutineFunctions[sh->NumSubroutineFunctions].types =
> ralloc_array(sh, const struct glsl_type *,
> fn->num_subroutine_types);
> +
> + /* From Section 4.4.4(Subroutine Function Layout Qualifiers) of the
> + * GLSL 4.5 spec:
> + *
> + * "Each subroutine with an index qualifier in the shader must be
> + * given a unique index, otherwise a compile or link error will be
> + * generated."
> + */
> + for (unsigned j = 0; j < sh->NumSubroutineFunctions; j++) {
> + if (sh->SubroutineFunctions[j].index != -1 &&
> + sh->SubroutineFunctions[j].index == fn->subroutine_index) {
> + linker_error(prog, "each subroutine index qualifier in the "
> + "shader must be unique\n");
> + return;
> + }
> + }
> + sh->SubroutineFunctions[sh->NumSubroutineFunctions].index =
> + fn->subroutine_index;
> +
> for (int j = 0; j < fn->num_subroutine_types; j++)
> sh->SubroutineFunctions[sh->NumSubroutineFunctions].types[j] = fn->subroutine_types[j];
> sh->NumSubroutineFunctions++;
> }
> +
> + /* Assign index for subroutines without an explicit index*/
> + int index = 0;
> + for (unsigned j = 0; j < sh->NumSubroutineFunctions; j++) {
> + while (sh->SubroutineFunctions[j].index == -1) {
> + for (unsigned k = 0; k < sh->NumSubroutineFunctions; k++) {
> + if (sh->SubroutineFunctions[k].index == index)
> + break;
> + else if (k == sh->NumSubroutineFunctions - 1)
> + sh->SubroutineFunctions[j].index = index;
> + }
> + index++;
> + }
> + }
> }
> }
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 4efdf1e..66820ca 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2193,6 +2193,7 @@ struct gl_ati_fragment_shader_state
> struct gl_subroutine_function
> {
> char *name;
> + int index;
> int num_compat_types;
> const struct glsl_type **types;
> };
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 58ba041..4b23c3a 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -661,6 +661,13 @@ _mesa_program_resource_index(struct gl_shader_program *shProg,
> switch (res->Type) {
> case GL_ATOMIC_COUNTER_BUFFER:
> return RESOURCE_ATC(res) - shProg->AtomicBuffers;
> + case GL_VERTEX_SUBROUTINE:
> + case GL_GEOMETRY_SUBROUTINE:
> + case GL_FRAGMENT_SUBROUTINE:
> + case GL_COMPUTE_SUBROUTINE:
> + case GL_TESS_CONTROL_SUBROUTINE:
> + case GL_TESS_EVALUATION_SUBROUTINE:
> + return RESOURCE_SUB(res)->index;
> case GL_UNIFORM_BLOCK:
> case GL_SHADER_STORAGE_BLOCK:
> case GL_TRANSFORM_FEEDBACK_VARYING:
>
More information about the mesa-dev
mailing list