On 10/2/18 7:38 PM, Vadym Shovkoplias wrote:
Hi Tapani,
Thanks for the review!
Completely agree with the first comment, I'll change that and resend the
patch.
Regarding second comment. I'm not sure if it is possible to do this
check after the optimization loop. From my observations compiler inlines
everything
and only after that it removes dead functions (actually all funcs except
"main"). After the optimization I don't see any possible way how to
implement this subroutine functions check because all functions and
functions signatures are removed at that point.
Yeah I was considering it could be done by storing some data but it
seems this is probably the most straightforward version.
On Tue, Oct 2, 2018 at 10:02 AM Tapani Pälli <[email protected]
<mailto:[email protected]>> wrote:
On 10/1/18 5:03 PM, Vadym Shovkoplias wrote:
> From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification
>
> "A program will fail to compile or link if any shader
> or stage contains two or more functions with the same
> name if the name is associated with a subroutine type."
>
> Fixes:
> * no-overloads.vert
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108109
> Signed-off-by: Vadym Shovkoplias
<[email protected]
<mailto:[email protected]>>
> ---
> src/compiler/glsl/linker.cpp | 40
++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/src/compiler/glsl/linker.cpp
b/src/compiler/glsl/linker.cpp
> index 3fde7e78d3..d0d017c7ff 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -4639,6 +4639,45 @@ link_assign_subroutine_types(struct
gl_shader_program *prog)
> }
> }
>
> +static void
> +verify_subroutine_associated_funcs(struct gl_shader_program *prog)
> +{
> + unsigned mask = prog->data->linked_stages;
> + while (mask) {
> + const int i = u_bit_scan(&mask);
> + gl_program *p = prog->_LinkedShaders[i]->Program;
> + glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;
> +
> + /*
> + * From OpenGL ES Shading Language 4.00 specification
> + * (6.1.2 Subroutines):
> + * "A program will fail to compile or link if any shader
> + * or stage contains two or more functions with the same
> + * name if the name is associated with a subroutine type."
> + */
> + for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {
> + unsigned definitions = 0;
> + char *name = p->sh.SubroutineFunctions[j].name;
> + ir_function *fn = symbols->get_function(name);
> +
> + /* Calculate number of function definitions with the
same name */
> + foreach_in_list(ir_function_signature, sig,
&fn->signatures) {
> + if (sig->is_defined)
> + definitions++;
You can just error out here, no need to calculate further.
I'm wondering a bit though should we fail here even if that function
was
not used at all (optimized out)? I can see that the Piglit test does
not
have a call to the function defined.
> + }
> +
> + if (definitions > 1) {
> + linker_error(prog, "%s shader contains %u function "
> + "definitions with name `%s', which is
associated with"
> + " a subroutine type.\n",
> + _mesa_shader_stage_to_string(i), definitions,
fn->name);
> + return;
> + }
> + }
> + }
> +}
> +
> +
> static void
> set_always_active_io(exec_list *ir, ir_variable_mode io_mode)
> {
> @@ -5024,6 +5063,7 @@ link_shaders(struct gl_context *ctx, struct
gl_shader_program *prog)
>
> check_explicit_uniform_locations(ctx, prog);
> link_assign_subroutine_types(prog);
> + verify_subroutine_associated_funcs(prog);
>
> if (!prog->data->LinkStatus)
> goto done;
>
_______________________________________________
mesa-dev mailing list
[email protected] <mailto:[email protected]>
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Vadym Shovkoplias | Senior Software Engineer
GlobalLogic
P +380.57.766.7667 M +3.8050.931.7304 S vadym.shovkoplias
www.globallogic.com <http://www.globallogic.com/>
<http://www.globallogic.com/>
http://www.globallogic.com/email_disclaimer.txt
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
