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 <sigles...@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 <apinhe...@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.shovkopl...@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@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
