On 21/08/18 11:42, Vadym Shovkoplias wrote: > 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 <[email protected] > <mailto:[email protected]>> > 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 ?
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 > > On Tue, Aug 21, 2018 at 12:00 PM, Alejandro Piñeiro > <[email protected] <mailto:[email protected]>> 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 > <https://bugs.freedesktop.org/show_bug.cgi?id=101247> > >> Signed-off-by: Vadym Shovkoplias > <[email protected] > <mailto:[email protected]>> > >> --- > >> 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 > > [email protected] > <mailto:[email protected]> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > <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/> > http://www.globallogic.com/email_disclaimer.txt
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
