<div dir="ltr"><div dir="ltr">Hi Iago,<div><br></div><div>I also think that compiler is the best place for the fix. But from my understanding compiler fix will be limited to distinct shader objects (not the shader stage).</div><div><div>In GLSL spec mentioned: "A program will fail to compile or link if any <b>shader or stage</b> contains two or more functions with the same name if the name is </div><div>associated with a subroutine type".</div></div><div>So if I understand the spec correctly such restriction for the shader stage can be fixed only in linker. E.g. consider the use case when fragment shader is linked from </div><div>two fragment shader objects. One object contains function foo() which is associated with subroutine type and second shader object has some regular foo() function </div><div>with the same name.  I suppose compiler fix won't be able to detect this.</div><div><br></div><div>IMHO the best way to fix this is to implement 2 patches:</div><div>1) One patch for linker to implement restriction for the shader stages (already done in this patch)</div><div>2) Another one for compiler to check the restriction for distinct shader objects.</div><div><br></div><div>What do you think ?</div></div></div><br><div class="gmail_quote"><div dir="ltr">ср, 3 окт. 2018 г. в 13:59, Iago Toral <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>>:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Vadym,<br>
<br>
I think this looks correct, but I was wondering if you considered<br>
implementing this check in ir_reader::read_function_sig (ir_reader.cpp)<br>
instead, which runs at compile time. My rationale is that given the<br>
option, I think it is preferable to push work to compile time rather<br>
than link time.<br>
<br>
Iago<br>
<br>
On Wed, 2018-10-03 at 11:39 +0300, 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>
> ++++++++++++++++++++++++++++++++++++<br>
>  1 file changed, 40 insertions(+)<br>
> <br>
> diff --git a/src/compiler/glsl/linker.cpp<br>
> b/src/compiler/glsl/linker.cpp<br>
> index 3fde7e78d3..aca5488a1e 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<br>
> 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<br>
> name */<br>
> +         foreach_in_list(ir_function_signature, sig, &fn-<br>
> >signatures) {<br>
> +            if (sig->is_defined) {<br>
> +               if (++definitions > 1) {<br>
> +                  linker_error(prog, "%s shader contains two or more<br>
> function "<br>
> +                        "definitions with name `%s', which is "<br>
> +                        "associated with a subroutine type.\n",<br>
> +                        _mesa_shader_stage_to_string(i),<br>
> +                        fn->name);<br>
> +                  return;<br>
> +               }<br>
> +            }<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<br>
> 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>
</blockquote></div>