<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    On 21/08/18 11:42, Vadym Shovkoplias wrote:<br>
    <blockquote type="cite"
cite="mid:CAKfM1-0HCAftoQUJewk5hVMp_kG5TVNYzNoKS6KT0spEeqOFHA@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      <div dir="ltr">Hi Timothy, Alejandro,
        <div><br>
        </div>
        <div>Thanks for the review comments! </div>
        <div>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: </div>
        <div>
          <div><br>
          </div>
        </div>
        <blockquote style="margin:0px 0px 0px
          40px;border:none;padding:0px">
          <div>
            <div>if (<b>input->data.used </b>&&
              !input->get_interface_type() &&</div>
          </div>
          <div>
            <div>                !input->data.explicit_location
              && !prog->SeparateShader)</div>
          </div>
          <div>
            <div>               linker_error(prog,</div>
          </div>
          <div>
            <div>                            "%s shader input `%s' "</div>
          </div>
          <div>
            <div>                            "has no matching output in
              the previous stage\n",</div>
          </div>
          <div>
            <div>                           
              _mesa_shader_stage_to_string(consumer->Stage),</div>
          </div>
          <div>
            <div>                            input->name);</div>
          </div>
        </blockquote>
        <br>
        This code is used for the same purpose to validate input and
        output variables which is also done during linking stage. 
        <div>So basically I just used the same check but for interface
          blocks.</div>
        <div><br>
        </div>
        <div>This was implemented some time ago in the following patch:</div>
        <div><br>
        </div>
        <blockquote style="margin:0 0 0 40px;border:none;padding:0px">
          <div>
            <div>commit 18004c338f6be8af2e36d2f54972c60136229aeb</div>
          </div>
          <div>
            <div>Author: Samuel Iglesias Gonsalvez <<a
                href="mailto:siglesias@igalia.com"
                moz-do-not-send="true">siglesias@igalia.com</a>></div>
          </div>
          <div>
            <div>Date:   Fri Nov 28 11:23:20 2014 +0100</div>
          </div>
          <div>
            <div><br>
            </div>
          </div>
          <div>
            <div>    glsl: fail when a shader's input var has not an
              equivalent out var in previous</div>
          </div>
        </blockquote>
        <div>
          <div><br>
          </div>
        </div>
        <div><br>
        </div>
        <div>Suggest please does this mean that it is safe to use "used"
          field while linking ?</div>
      </div>
    </blockquote>
    <br>
    Well, it seems so. Or at least it was already being used as that,
    and have been working since 2014. Then I think that it would be ok
    to use it for your patch, but I still think that in this case it
    would be needed to slightly tweak the comment at ir.h<br>
    <br>
    <blockquote type="cite"
cite="mid:CAKfM1-0HCAftoQUJewk5hVMp_kG5TVNYzNoKS6KT0spEeqOFHA@mail.gmail.com">
      <div class="gmail_extra"><br>
        <div class="gmail_quote">On Tue, Aug 21, 2018 at 12:00 PM,
          Alejandro Piñeiro <span dir="ltr"><<a
              href="mailto:apinheiro@igalia.com" target="_blank"
              moz-do-not-send="true">apinheiro@igalia.com</a>></span>
          wrote:<br>
          <blockquote class="gmail_quote" style="margin:0 0 0
            .8ex;border-left:1px #ccc solid;padding-left:1ex">
            <div class="HOEnZb">
              <div class="h5">On 21/08/18 03:02, Timothy Arceri wrote:<br>
                > On 20/08/18 23:31, vadym.shovkoplias wrote:<br>
                >>  From Section 4.3.4 (Inputs) of the GLSL 1.50
                spec:<br>
                >><br>
                >>      "Only the input variables that are
                actually read need to be written<br>
                >>       by the previous stage; it is allowed to
                have superfluous<br>
                >>       declarations of input variables."<br>
                >><br>
                >> Fixes:<br>
                >>      * interstage-multiple-shader-<wbr>objects.shader_test<br>
                >><br>
                >> Bugzilla: <a
                  href="https://bugs.freedesktop.org/show_bug.cgi?id=101247"
                  rel="noreferrer" target="_blank"
                  moz-do-not-send="true">https://bugs.freedesktop.org/<wbr>show_bug.cgi?id=101247</a><br>
                >> Signed-off-by: Vadym Shovkoplias <<a
                  href="mailto:vadym.shovkoplias@globallogic.com"
                  moz-do-not-send="true">vadym.shovkoplias@<wbr>globallogic.com</a>><br>
                >> ---<br>
                >>   src/compiler/glsl/link_<wbr>interface_blocks.cpp
                | 8 +++++++-<br>
                >>   1 file changed, 7 insertions(+), 1
                deletion(-)<br>
                >><br>
                >> diff --git a/src/compiler/glsl/link_<wbr>interface_blocks.cpp<br>
                >> b/src/compiler/glsl/link_<wbr>interface_blocks.cpp<br>
                >> index e5eca9460e..801fbcd5d9 100644<br>
                >> --- a/src/compiler/glsl/link_<wbr>interface_blocks.cpp<br>
                >> +++ b/src/compiler/glsl/link_<wbr>interface_blocks.cpp<br>
                >> @@ -417,9 +417,15 @@ validate_interstage_inout_<wbr>blocks(struct<br>
                >> gl_shader_program *prog,<br>
                >>          * write to any of the pre-defined
                outputs (e.g. if the<br>
                >> vertex shader<br>
                >>          * does not write to gl_Position, etc),
                which is allowed and<br>
                >> results in<br>
                >>          * undefined behavior.<br>
                >> +       *<br>
                >> +       * From Section 4.3.4 (Inputs) of the
                GLSL 1.50 spec:<br>
                >> +       *<br>
                >> +       *    "Only the input variables that are
                actually read need to<br>
                >> be written<br>
                >> +       *     by the previous stage; it is
                allowed to have superfluous<br>
                >> +       *     declarations of input variables."<br>
                >>          */<br>
                >>         if (producer_def == NULL &&<br>
                >> -          !is_builtin_gl_in_block(var,
                consumer->Stage)) {<br>
                >> +          !is_builtin_gl_in_block(var,
                consumer->Stage) &&<br>
                >> var->data.used) {<br>
                ><br>
                > This concerns me a little. As far as I remember
                'used' was added to<br>
                > make compiler warning better but it's not 100%
                reliable.<br>
                <br>
              </div>
            </div>
            +1 on the concerns thing. In addition to be used mostly for
            warnings, we<br>
            need to take into account his description comment at ir.h:<br>
            <br>
                 "<br>
                   * This is only maintained in the ast_to_hir.cpp path,
            not in<br>
                   * Mesa's fixed function or ARB program paths.<br>
                 "<br>
            <br>
            So if we start to use this while linking, then we need to
            ensure that it<br>
            is maintained outside the ast_to_hir pass (or at least,
            ensure that it<br>
            is correct after that pass). And if we got that, then that
            comment<br>
            became obsolete, and should be removed as part of the patch.<br>
            <span class="im HOEnZb"><br>
              ><br>
              >>            linker_error(prog, "Input block `%s'
              is not an output of "<br>
              >>                         "the previous stage\n",<br>
              >> var->get_interface_type()-><wbr>name);<br>
              >>            return;<br>
              >><br>
            </span>
            <div class="HOEnZb">
              <div class="h5">> ______________________________<wbr>_________________<br>
                > mesa-dev mailing list<br>
                > <a href="mailto:mesa-dev@lists.freedesktop.org"
                  moz-do-not-send="true">mesa-dev@lists.freedesktop.org</a><br>
                > <a
                  href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev"
                  rel="noreferrer" target="_blank"
                  moz-do-not-send="true">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
                <br>
              </div>
            </div>
          </blockquote>
        </div>
        <br>
        <br clear="all">
        <div><br>
        </div>
        -- <br>
        <div class="gmail_signature" data-smartmail="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"
                                moz-do-not-send="true">+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" moz-do-not-send="true"><span
style="font-size:12px;font-family:Arial;color:#1155cc;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" moz-do-not-send="true"><span
style="font-size:12px;font-family:Arial;color:#1155cc;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" moz-do-not-send="true"><span
style="font-size:11px;font-family:Arial;color:#1155cc;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>
    </blockquote>
    <br>
  </body>
</html>