<div dir="ltr"><div dir="ltr">Hi Tapani,<div><br></div><div>Thanks for the review!</div><div><br></div><div>Completely agree with the first comment, I'll change that and resend the patch.</div><div>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</div><div>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 </div><div>implement this subroutine functions check because all functions and functions signatures are removed at that point. </div><div><br><div class="gmail_quote"><div dir="ltr">On Tue, Oct 2, 2018 at 10:02 AM Tapani Pälli <<a href="mailto:tapani.palli@intel.com">tapani.palli@intel.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
On 10/1/18 5:03 PM, Vadym Shovkoplias wrote:<br>
> From Section 6.1.2 (Subroutines) of the GLSL 4.00 specification<br>
> <br>
> "A program will fail to compile or link if any shader<br>
> or stage contains two or more functions with the same<br>
> name if the name is associated with a subroutine type."<br>
> <br>
> Fixes:<br>
> * no-overloads.vert<br>
> <br>
> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=108109" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=108109</a><br>
> Signed-off-by: Vadym Shovkoplias <<a href="mailto:vadym.shovkoplias@globallogic.com" target="_blank">vadym.shovkoplias@globallogic.com</a>><br>
> ---<br>
> src/compiler/glsl/linker.cpp | 40 ++++++++++++++++++++++++++++++++++++<br>
> 1 file changed, 40 insertions(+)<br>
> <br>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp<br>
> index 3fde7e78d3..d0d017c7ff 100644<br>
> --- a/src/compiler/glsl/linker.cpp<br>
> +++ b/src/compiler/glsl/linker.cpp<br>
> @@ -4639,6 +4639,45 @@ link_assign_subroutine_types(struct gl_shader_program *prog)<br>
> }<br>
> }<br>
> <br>
> +static void<br>
> +verify_subroutine_associated_funcs(struct gl_shader_program *prog)<br>
> +{<br>
> + unsigned mask = prog->data->linked_stages;<br>
> + while (mask) {<br>
> + const int i = u_bit_scan(&mask);<br>
> + gl_program *p = prog->_LinkedShaders[i]->Program;<br>
> + glsl_symbol_table *symbols = prog->_LinkedShaders[i]->symbols;<br>
> +<br>
> + /*<br>
> + * From OpenGL ES Shading Language 4.00 specification<br>
> + * (6.1.2 Subroutines):<br>
> + * "A program will fail to compile or link if any shader<br>
> + * or stage contains two or more functions with the same<br>
> + * name if the name is associated with a subroutine type."<br>
> + */<br>
> + for (unsigned j = 0; j < p->sh.NumSubroutineFunctions; j++) {<br>
> + unsigned definitions = 0;<br>
> + char *name = p->sh.SubroutineFunctions[j].name;<br>
> + ir_function *fn = symbols->get_function(name);<br>
> +<br>
> + /* Calculate number of function definitions with the same name */<br>
> + foreach_in_list(ir_function_signature, sig, &fn->signatures) {<br>
> + if (sig->is_defined)<br>
> + definitions++;<br>
<br>
You can just error out here, no need to calculate further.<br>
<br>
I'm wondering a bit though should we fail here even if that function was <br>
not used at all (optimized out)? I can see that the Piglit test does not <br>
have a call to the function defined.<br>
<br>
<br>
> + }<br>
> +<br>
> + if (definitions > 1) {<br>
> + linker_error(prog, "%s shader contains %u function "<br>
> + "definitions with name `%s', which is associated with"<br>
> + " a subroutine type.\n",<br>
> + _mesa_shader_stage_to_string(i), definitions, fn->name);<br>
> + return;<br>
> + }<br>
> + }<br>
> + }<br>
> +}<br>
> +<br>
> +<br>
> static void<br>
> set_always_active_io(exec_list *ir, ir_variable_mode io_mode)<br>
> {<br>
> @@ -5024,6 +5063,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)<br>
> <br>
> check_explicit_uniform_locations(ctx, prog);<br>
> link_assign_subroutine_types(prog);<br>
> + verify_subroutine_associated_funcs(prog);<br>
> <br>
> if (!prog->data->LinkStatus)<br>
> goto done;<br>
> <br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><font size="-1"><br><span style="vertical-align:baseline;font-variant:normal;font-style:normal;font-size:12px;background-color:transparent;text-decoration:none;font-family:Arial;font-weight:bold">Vadym Shovkoplias | Senior Software Engineer</span><br><span style="vertical-align:baseline;font-variant:normal;font-style:normal;font-size:12px;background-color:transparent;text-decoration:none;font-family:Arial;font-weight:normal">GlobalLogic</span><br><span style="vertical-align:baseline;font-variant:normal;font-style:normal;font-size:12px;background-color:transparent;text-decoration:none;font-family:Arial;font-weight:normal"></span></font><font size="-1"><span style="vertical-align:baseline;font-variant:normal;font-style:normal;font-size:12px;background-color:transparent;text-decoration:none;font-family:Arial;font-weight:normal"><span><font color="#888888"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px">P </span><a style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px">+380.57.766.7667</a></font></span> M +3.8050.931.7304 S vadym.shovkoplias</span><br><a href="http://www.globallogic.com/" target="_blank"><span style="font-size:12px;font-family:Arial;color:rgb(17,85,204);background-color:transparent;font-weight:normal;font-style:normal;font-variant:normal;text-decoration:underline;vertical-align:baseline">www.globallogic.com</span></a><span style="vertical-align:baseline;font-variant:normal;font-style:normal;font-size:12px;background-color:transparent;text-decoration:none;font-family:Arial;font-weight:normal"></span><br><a href="http://www.globallogic.com/" target="_blank"><span style="font-size:12px;font-family:Arial;color:rgb(17,85,204);background-color:transparent;font-weight:normal;font-style:normal;font-variant:normal;text-decoration:underline;vertical-align:baseline"></span></a><br><a href="http://www.globallogic.com/email_disclaimer.txt" target="_blank"><span style="font-size:11px;font-family:Arial;color:rgb(17,85,204);background-color:transparent;font-weight:normal;font-style:normal;font-variant:normal;text-decoration:underline;vertical-align:baseline">http://www.globallogic.com/email_disclaimer.txt</span></a><span style="vertical-align:baseline;font-variant:normal;font-style:normal;font-size:11px;background-color:transparent;text-decoration:none;font-family:Arial;font-weight:normal"></span></font></div></div></div></div></div></div></div></div></div></div>