[Mesa-dev] [PATCH] glsl/linker: Check the subroutine associated functions names

[Iago Toral]

Hi Vadym,

I think this looks correct, but I was wondering if you considered
implementing this check in ir_reader::read_function_sig (ir_reader.cpp)
instead, which runs at compile time. My rationale is that given the
option, I think it is preferable to push work to compile time rather
than link time.

Iago

On Wed, 2018-10-03 at 11:39 +0300, 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 <vadym.shovkoplias at globallogic.com>
> ---
>  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..aca5488a1e 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) {
> +               if (++definitions > 1) {
> +                  linker_error(prog, "%s shader contains two or more
> function "
> +                        "definitions with name `%s', which is "
> +                        "associated with a subroutine type.\n",
> +                        _mesa_shader_stage_to_string(i),
> +                        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;