[Mesa-dev] [PATCH] glsl/linker: Allow unused in blocks which are not declated on previous stage

[Vadym Shovkoplias]

Hi Timothy, Alejandro,

Thanks for the review comments!
I'd just like to mention that the same approach is already used
in link_varyings.cpp file in function cross_validate_outputs_to_inputs().
Here is a code snippet:

if (*input->data.used *&& !input->get_interface_type() &&
                !input->data.explicit_location && !prog->SeparateShader)
               linker_error(prog,
                            "%s shader input `%s' "
                            "has no matching output in the previous
stage\n",
                            _mesa_shader_stage_to_string(consumer->Stage),
                            input->name);


This code is used for the same purpose to validate input and output
variables which is also done during linking stage.
So basically I just used the same check but for interface blocks.

This was implemented some time ago in the following patch:

commit 18004c338f6be8af2e36d2f54972c60136229aeb
Author: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
Date:   Fri Nov 28 11:23:20 2014 +0100

    glsl: fail when a shader's input var has not an equivalent out var in
previous



Suggest please does this mean that it is safe to use "used" field while
linking ?

On Tue, Aug 21, 2018 at 12:00 PM, Alejandro Piñeiro <apinheiro at igalia.com>
wrote:

> On 21/08/18 03:02, Timothy Arceri wrote:
> > On 20/08/18 23:31, vadym.shovkoplias wrote:
> >>  From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
> >>
> >>      "Only the input variables that are actually read need to be written
> >>       by the previous stage; it is allowed to have superfluous
> >>       declarations of input variables."
> >>
> >> Fixes:
> >>      * interstage-multiple-shader-objects.shader_test
> >>
> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101247
> >> Signed-off-by: Vadym Shovkoplias <vadym.shovkoplias at globallogic.com>
> >> ---
> >>   src/compiler/glsl/link_interface_blocks.cpp | 8 +++++++-
> >>   1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/compiler/glsl/link_interface_blocks.cpp
> >> b/src/compiler/glsl/link_interface_blocks.cpp
> >> index e5eca9460e..801fbcd5d9 100644
> >> --- a/src/compiler/glsl/link_interface_blocks.cpp
> >> +++ b/src/compiler/glsl/link_interface_blocks.cpp
> >> @@ -417,9 +417,15 @@ validate_interstage_inout_blocks(struct
> >> gl_shader_program *prog,
> >>          * write to any of the pre-defined outputs (e.g. if the
> >> vertex shader
> >>          * does not write to gl_Position, etc), which is allowed and
> >> results in
> >>          * undefined behavior.
> >> +       *
> >> +       * From Section 4.3.4 (Inputs) of the GLSL 1.50 spec:
> >> +       *
> >> +       *    "Only the input variables that are actually read need to
> >> be written
> >> +       *     by the previous stage; it is allowed to have superfluous
> >> +       *     declarations of input variables."
> >>          */
> >>         if (producer_def == NULL &&
> >> -          !is_builtin_gl_in_block(var, consumer->Stage)) {
> >> +          !is_builtin_gl_in_block(var, consumer->Stage) &&
> >> var->data.used) {
> >
> > This concerns me a little. As far as I remember 'used' was added to
> > make compiler warning better but it's not 100% reliable.
>
> +1 on the concerns thing. In addition to be used mostly for warnings, we
> need to take into account his description comment at ir.h:
>
>      "
>        * This is only maintained in the ast_to_hir.cpp path, not in
>        * Mesa's fixed function or ARB program paths.
>      "
>
> So if we start to use this while linking, then we need to ensure that it
> is maintained outside the ast_to_hir pass (or at least, ensure that it
> is correct after that pass). And if we got that, then that comment
> became obsolete, and should be removed as part of the patch.
>
> >
> >>            linker_error(prog, "Input block `%s' is not an output of "
> >>                         "the previous stage\n",
> >> var->get_interface_type()->name);
> >>            return;
> >>
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > 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/email_disclaimer.txt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180821/4eb8c90c/attachment.html>