[Mesa-dev] [PATCH 3/3] glsl/linker: Check that re-declared, inter-shader built-in blocks match

[Timothy Arceri]

On Fri, 2017-01-27 at 17:42 +0100, Eduardo Lima Mitev wrote:
> From GLSL 4.5 spec, section "7.1 Built-In Language Variables", page
> 130 of
> the PDF states:
> 
>     "If multiple shaders using members of a built-in block belonging
> to
>      the same interface are linked together in the same program, they
> must
>      all redeclare the built-in block in the same way, as described
> in
>      section 4.3.9 “Interface Blocks” for interface-block matching,
> or a
>      link-time error will result."
> 
> Fixes:
> * GL45-CTS.CommonBugs.CommonBug_PerVertexValidation


I was looking at this test yesterday and noticed the the GS input is
not used, so technically there is no reason the builtin can't be
optimised away. This means there is no need for validation between the
GS interface and whatever output it is linked against, and is IMO a bug
in the test.

Therefore I'm concerned that this series is forcing validation on
unused varyings which isn't required by the spec.


> ---
>  src/compiler/glsl/link_interface_blocks.cpp | 33
> ++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compiler/glsl/link_interface_blocks.cpp
> b/src/compiler/glsl/link_interface_blocks.cpp
> index 7037c7776de..4c0278661fa 100644
> --- a/src/compiler/glsl/link_interface_blocks.cpp
> +++ b/src/compiler/glsl/link_interface_blocks.cpp
> @@ -376,11 +376,42 @@ validate_interstage_inout_blocks(struct
> gl_shader_program *prog,
>     /* Verify that the consumer's input interfaces match. */
>     foreach_in_list(ir_instruction, node, consumer->ir) {
>        ir_variable *var = node->as_variable();
> -      if (!var || !var->get_interface_type() || var->data.mode !=
> ir_var_shader_in)
> +      if (!var || !var->get_interface_type())
>           continue;
>  
>        ir_variable *producer_def = definitions.lookup(var);
>  
> +      /* Check that all built-in block re-declarations are
> compatible
> +       * across shaders: From OpenGL Shading Language 4.5, section
> +       * "7.1 Built-In Language Variables", page 130 of the PDF:
> +       *
> +       *    "If multiple shaders using members of a built-in block
> belonging
> +       *     to the same interface are linked together in the same
> program,
> +       *     they must all redeclare the built-in block in the same
> way, as
> +       *     described in section 4.3.9 “Interface Blocks” for
> interface-block
> +       *     matching, or a link-time error will result."
> +       */
> +      const glsl_type *consumer_iface =
> +         consumer->symbols->get_interface(var->get_interface_type()-
> >name,
> +                                          ir_var_shader_in);
> +
> +      const glsl_type *producer_iface = NULL;
> +      if (producer_def && producer_def->get_interface_type()) {
> +         producer_iface =
> +            producer->symbols->get_interface(producer_def-
> >get_interface_type()->name,
> +                                             ir_var_shader_out);
> +      }
> +
> +      if (producer_iface && consumer_iface &&
> +          interstage_member_mismatch(prog, consumer_iface,
> producer_iface)) {
> +         linker_error(prog, "Incompatible or missing gl_PerVertex
> re-declaration"
> +                      "in consecutive shaders");
> +         return;
> +      }
> +
> +      if (var->data.mode != ir_var_shader_in)
> +         continue;
> +
>        /* The producer doesn't generate this input: fail to link.
> Skip built-in
>         * 'gl_in[]' since that may not be present if the producer
> does not
>         * write to any of the pre-defined outputs (e.g. if the vertex
> shader