Re: [Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs
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 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 > > > > Cc: Iago Toral Quiroga > > > > Cc: Samuel Iglesias Gonsálvez > > > > Cc: Tapani Pälli > > > > Cc: Ian Romanick > > > > Signed-off-by: Andres Gomez > > > > --- > > > > 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 " > > > > +
Re: [Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs
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 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 Cc: Iago Toral Quiroga Cc: Samuel Iglesias Gonsálvez Cc: Tapani Pälli Cc: Ian Romanick Signed-off-by: Andres Gomez --- 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. 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. + 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs
On Fri, 2019-02-01 at 18:37 -0500, Ilia Mirkin wrote: > On Fri, Feb 1, 2019 at 1:08 PM Andres Gomez 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 > > Cc: Iago Toral Quiroga > > Cc: Samuel Iglesias Gonsálvez > > Cc: Tapani Pälli > > Cc: Ian Romanick > > Signed-off-by: Andres Gomez > > --- > > 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. 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") > > > + 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
Re: [Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs
On 2/2/19 10:28 am, Timothy Arceri wrote: On 2/2/19 5:05 am, Andres Gomez 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 Cc: Iago Toral Quiroga Cc: Samuel Iglesias Gonsálvez Cc: Tapani Pälli Cc: Ian Romanick Signed-off-by: Andres Gomez --- 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) { This is not really what used was designed for so it's always a bit unsettling to see this type of thing. However its better that what we do now and is consistent with 18004c338f6 so this patch is: Reviewed-by: Timothy Arceri Actually I take this back after seeing Ilia's comment. I think we need some more piglit tests for the SSO cases where an SSO contains two or more stages. + 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", ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs
On Fri, Feb 1, 2019 at 1:08 PM Andres Gomez 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 > Cc: Iago Toral Quiroga > Cc: Samuel Iglesias Gonsálvez > Cc: Tapani Pälli > Cc: Ian Romanick > Signed-off-by: Andres Gomez > --- > 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? > + 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs
On 2/2/19 5:05 am, Andres Gomez 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 Cc: Iago Toral Quiroga Cc: Samuel Iglesias Gonsálvez Cc: Tapani Pälli Cc: Ian Romanick Signed-off-by: Andres Gomez --- 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) { This is not really what used was designed for so it's always a bit unsettling to see this type of thing. However its better that what we do now and is consistent with 18004c338f6 so this patch is: Reviewed-by: Timothy Arceri + 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", ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/6] glsl/linker: don't fail non static used inputs without matching outputs
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 Cc: Iago Toral Quiroga Cc: Samuel Iglesias Gonsálvez Cc: Tapani Pälli Cc: Ian Romanick Signed-off-by: Andres Gomez --- 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) { + 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