From: Timothy Arceri <timothy.arc...@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 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(-) diff --git a/src/glsl/ast.h b/src/glsl/ast.h index 8fbb5dd..f96585f 100644 --- a/src/glsl/ast.h +++ b/src/glsl/ast.h @@ -771,7 +771,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 e71c539..f1cc263 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -2617,28 +2617,37 @@ validate_layout_qualifiers(const struct ast_type_qualifier *qual, validate_explicit_location(qual, var, state, loc); if (qual->flags.q.explicit_index) { - /* From the GLSL 4.30 specification, section 4.4.2 (Output - * Layout Qualifiers): - * - * "It is also a compile-time error if a fragment shader - * sets a layout index to less than 0 or greater than 1." - * - * Older specifications don't mandate a behavior; we take - * this as a clarification and always generate the error. - */ - int qual_index; - if (process_qualifier_constant(state, loc, "index", - qual->index, &qual_index) && - (qual_index < 0 || qual_index > 1)) { - _mesa_glsl_error(loc, state, - "explicit index may only be 0 or 1"); + /* Check if index was set for the uniform instead of the function */ + if (qual->flags.q.subroutine) { + _mesa_glsl_error(loc, state, "an index qualifier can only be " + "used with subroutine functions"); } else { - var->data.explicit_index = true; - var->data.index = qual_index; + + /* From the GLSL 4.30 specification, section 4.4.2 (Output + * Layout Qualifiers): + * + * "It is also a compile-time error if a fragment shader + * sets a layout index to less than 0 or greater than 1." + * + * Older specifications don't mandate a behavior; we take + * this as a clarification and always generate the error. + */ + int qual_index; + if (process_qualifier_constant(state, loc, "index", + qual->index, &qual_index) && + (qual_index < 0 || qual_index > 1)) { + _mesa_glsl_error(loc, state, + "explicit index may only be 0 or 1"); + } else { + var->data.explicit_index = true; + var->data.index = qual_index; + } } } } 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 && @@ -4855,7 +4864,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); @@ -4987,6 +4996,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) { + int 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 < 0 || + qual_index > (MAX_SUBROUTINES - 1)) { + _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 d90b473..2931b0c 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; } @@ -175,6 +178,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 9c9f22d..97cbcc3 100644 --- a/src/glsl/ir.h +++ b/src/glsl/ir.h @@ -1157,6 +1157,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 26c0298..7697c78 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 fdb3b3d..f1e3319 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 5cb877b..c788f26 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: -- 2.4.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev