I agree that this comment is obsolete now. I'll update the patch, thanks! On Wed, Aug 22, 2018 at 12:09 PM, Alejandro Piñeiro <apinhe...@igalia.com> wrote:
> 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 <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 ? > > > 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 <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 > > > -- 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
