On Wed, 2019-02-06 at 09:42 +1100, Timothy Arceri wrote:
> On 6/2/19 1:11 am, Andres Gomez wrote:
> > On Fri, 2019-02-01 at 18:37 -0500, Ilia Mirkin wrote:
> > > On Fri, Feb 1, 2019 at 1:08 PM Andres Gomez <ago...@igalia.com> wrote:
> > > > If there is no Static Use of an input variable, the linker shouldn't
> > > > fail whenever there is no defined matching output variable in the
> > > > previous stage.
> > > >
> > > > From page 47 (page 51 of the PDF) of the GLSL 4.60 v.5 spec:
> > > >
> > > > " Only the input variables that are statically read need to be
> > > > written by the previous stage; it is allowed to have superfluous
> > > > declarations of input variables."
> > > >
> > > > Now, we complete this exception whenever the input variable has an
> > > > explicit location. Previously, 18004c338f6 ("glsl: fail when a
> > > > shader's input var has not an equivalent out var in previous") took
> > > > care of the cases in which the input variable didn't have an explicit
> > > > location.
> > > >
> > > > Additionally, likewise 1aa5738e666 ("glsl: relax input->output
> > > > validation for SSO programs"), avoid failing also for programs that
> > > > utilize GL_ARB_separate_shader_objects.
> > > >
> > > > Cc: Timothy Arceri <tarc...@itsqueeze.com>
> > > > Cc: Iago Toral Quiroga <ito...@igalia.com>
> > > > Cc: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
> > > > Cc: Tapani Pälli <tapani.pa...@intel.com>
> > > > Cc: Ian Romanick <ian.d.roman...@intel.com>
> > > > Signed-off-by: Andres Gomez <ago...@igalia.com>
> > > > ---
> > > > src/compiler/glsl/link_varyings.cpp | 16 ++++++++++++++--
> > > > 1 file changed, 14 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/src/compiler/glsl/link_varyings.cpp
> > > > b/src/compiler/glsl/link_varyings.cpp
> > > > index e5f7d3e322a..6cebc5b3c5a 100644
> > > > --- a/src/compiler/glsl/link_varyings.cpp
> > > > +++ b/src/compiler/glsl/link_varyings.cpp
> > > > @@ -808,8 +808,20 @@ cross_validate_outputs_to_inputs(struct gl_context
> > > > *ctx,
> > > >
> > > > output =
> > > > output_explicit_locations[idx][input->data.location_frac].var;
> > > >
> > > > - if (output == NULL ||
> > > > - input->data.location != output->data.location) {
> > > > + if (output == NULL) {
> > > > + /* A linker failure should only happen when, for
> > > > programs
> > > > + * not using sso, there is no output declaration and
> > > > there
> > > > + * is Static Use of the declared input.
> > > > + */
> > > > + if (input->data.used && !prog->SeparateShader) {
> > >
> > > Should this differentiate whether this is the first stage of a
> > > separable program vs a later one? Presumably the exception only
> > > applies at the separate program boundary, not to each shader within
> > > the program?
> >
> > Cc-ing Tapani and Anuj.
> >
> > As I understand the spec, it applies to every shader within the
> > separable program.
>
> No that's not what the spec says. From the quote below:
>
> "With separable program objects, interfaces between shader stages
> may involve the outputs from one program object and the inputs from a
> second program object."
>
> So if we have two "program" objects say:
>
> 1. vs->gs
> 2. fs
>
> The relaxed restrictions only apply to the gs -> fs interface. Not the
> vs -> gs interface.
After double checking specs and examples, I think you are right.
I'll update the patch and send a v2 shortly.
>
>
> > From the ARB_separate_shader_objects spec:
> >
> > " With separable program objects, interfaces between shader stages
> > may involve the outputs from one program object and the inputs
> > from a second program object. For such interfaces, it is not
> > possible to detect mismatches at link time, because the programs
> > are linked separately. When each such program is linked, all
> > inputs or outputs interfacing with another program stage are
> > treated as active. The linker will generate an executable that
> > assumes the presence of a compatible program on the other side of
> > the interface. If a mismatch between programs occurs, no GL error
> > will be generated, but some or all of the inputs on the interface
> > will be undefined."
> >
> > Notice that this has also been the interpretation from:
> >
> > 1aa5738e666 ("glsl: relax input->output validation for SSO programs")
>
> Yes this looks wrong also. We should write some piglit tests for this
> and fix it too.
I'll address reverting that commit later.
Doing so makes "arb_program_interface_query-getprogramresourceiv" and
"arb_program_interface_query-resource-query" piglit tests fail so it
seems I'll have to fix those tests and add some more to check the
interface matching in the internal interfaces of separable programs.
>
> > > > + linker_error(prog,
> > > > + "%s shader input `%s' with explicit
> > > > location "
> > > > + "has no matching output\n",
> > > > +
> > > > _mesa_shader_stage_to_string(consumer->Stage),
> > > > + input->name);
> > > > + break;
> > > > + }
> > > > + } else if (input->data.location !=
> > > > output->data.location) {
> > > > linker_error(prog,
> > > > "%s shader input `%s' with explicit
> > > > location "
> > > > "has no matching output\n",
> > > > --
> > > > 2.20.1
> > > >
> > > > _______________________________________________
> > > > mesa-dev mailing list
> > > > mesa-dev@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Br,
Andres
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
