Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Kenneth Graunke]

On Monday, December 19, 2016 8:41:17 PM PST Matt Turner wrote:
> On Mon, Dec 19, 2016 at 4:13 PM, Kenneth Graunke  
> wrote:
> > For what it's worth, the OpenGL wiki's Program Introspection page(*),
> > under "Interface block member naming" gives an example matching my above
> > reply.  It says:
> >
> > uniform BlockName3
> > {
> >   int mem;
> > } instanceName3[4];
> >
> > This definition will create a single member named "BlockName3.min".
> > The reason this array of four blocks only counts as having one
> > variable is because each of the four blocks uses the same internal
> > definition. There is nothing that could be queried from
> > BlockName3[1] that could not be queried from BlockName3[0].
> >
> > (*) https://www.khronos.org/opengl/wiki/Program_Introspection
> >
> > I think that's a decent explanation of why this is reasonable.  Because
> > the block entries have per-element entries (Block[0], Block[1], etc.
> > you can query whether a block is referenced (i.e. a UBO binding is used).
> 
> This might be a stupid question, but why is the field named "mem" in
> the code, but the following paragraph says "min"? Are the two not
> supposed to be the same?

Oh.  I think that's a typo.  It should be BlockName3.mem.


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Matt Turner]

On Mon, Dec 19, 2016 at 4:13 PM, Kenneth Graunke  wrote:
> For what it's worth, the OpenGL wiki's Program Introspection page(*),
> under "Interface block member naming" gives an example matching my above
> reply.  It says:
>
> uniform BlockName3
> {
>   int mem;
> } instanceName3[4];
>
> This definition will create a single member named "BlockName3.min".
> The reason this array of four blocks only counts as having one
> variable is because each of the four blocks uses the same internal
> definition. There is nothing that could be queried from
> BlockName3[1] that could not be queried from BlockName3[0].
>
> (*) https://www.khronos.org/opengl/wiki/Program_Introspection
>
> I think that's a decent explanation of why this is reasonable.  Because
> the block entries have per-element entries (Block[0], Block[1], etc.
> you can query whether a block is referenced (i.e. a UBO binding is used).

This might be a stupid question, but why is the field named "mem" in
the code, but the following paragraph says "min"? Are the two not
supposed to be the same?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Kenneth Graunke]

On Monday, December 19, 2016 1:36:00 PM PST Ian Romanick wrote:
> On 12/16/2016 09:35 PM, Kenneth Graunke wrote:
> > This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
> > arb_program_interface_query/arb_program_interface_query-resource-query,
> > and GL45-CTS.program_interface_query.separate-programs-{tess-control,
> > tess-eval,geometry}.  Only one dEQP program interface failure remains.
> > 
> > I would have liked to split this up into several distinct changes, but
> > I wasn't sure how to do that given thet tangled nature of these issues.
> > 
> > So, the issues:
> > 
> >* We need to treat interface blocks declared as an array of instances
> >  as a single block - removing the outer array.  The resource list
> >  entry's name should not include the array length.  Properties such
> >  as GL_ARRAY_SIZE should refer to the variable inside the block, not
> >  the interface block's array properties.
> > 
> >* We need to do this prefixing even for structure variables.
> > 
> >* We need to do this for built-ins (such as gl_PerVertex.gl_Position).
> > 
> >* After interface array unwrapping, any variable which is an array
> >  should have [0] appended.  It doesn't matter if it's a TCS/TES/GS
> >  input or TCS output - that looked like an attempt to unwrap for
> >  per-vertex variables, but that didn't consider per-patch variables,
> >  and as far as I can tell there's nothing to justify this.
> > 
> > Several Mesa developers have suggested that Issue 16 contradicts the
> > main specification, but I believe that it doesn't - the main spec just
> > isn't terribly clear.  The main ARB_program_interface query spec says:
> > 
> >   "* For an active interface block not declared as an array of block
> >  instances, a single entry will be generated, using the block name from
> >  the shader source.
> > 
> >* For an active interface block declared as an array of instances,
> >  separate entries will be generated for each active instance.  The name
> >  of the instance is formed by concatenating the block name, the "["
> >  character, an integer identifying the instance number, and the "]"
> >  character."
> > 
> > Issue 16 says that built-ins should be named "gl_PerVertex.gl_Position",
> > but several people suggested the second bullet above means that it
> > should be named "gl_PerVertex[array length].gl_Position".
> > 
> > There are two important things to note.  Those bullet points say
> > "an active interface block", while the others say "variable" or "active
> > shader storage block member".  They also don't mention applying the
> > rules recursively (unlike the other bullets).  Both suggest that
> > these rules apply to blocks themselves, not members of blocks.
> > 
> > In fact, for GL_UNIFORM_BLOCK queries, we do have "block[0]",
> > "block[1]", ... resource list entries - so those rules are real,
> > and actually used.  So if they don't apply to block members, then how
> > should members be named?  Unfortunately, I don't see any rules outside
> > of issue 16 - where the rationale is very unclear.  I hope to clarify
> > the spec in the future.
> 
> Based on my understanding, something like
> 
> out Vertex {
> vec4 p;
> vec4 c[3];
> } vertex[2];
> 
> in a vertex shader should produce
> 
> Vertex[0].p
> Vertex[0].c[0] (with GL_ARRAY_SIZE = 3)
> Vertex[1].p
> Vertex[1].c[0] (with GL_ARRAY_SIZE = 3)
> 
> This is definitely what we would produce for a uniform block.
> 
> What I've never understood is why that isn't done for gl_PerVertex stage
> boundaries where gl_PerVertex is explicitly arrayed.

My expectation is that the above code would produce:

Block entries:

Vertex[0]
Vertex[1]

Variable/member entries:

Vertex.p
Vertex.c[0] (with GL_ARRAY_SIZE = 3)

We would produce similar results for uniform blocks after my patch.

I don't think anything about the rules is gl_PerVertex specific.

> I think this patch is good for now.  I like that it brings all the
> strange edge-case handling into one place.
> 
> Reviewed-by: Ian Romanick 

I've attached some notes I took while looking at dEQP program interface
query tests - it has the shader code and expected results for a lot of
test cases.  I didn't write down the uniform ones, but they follow the
same pattern (as shown above).

For what it's worth, the OpenGL wiki's Program Introspection page(*),
under "Interface block member naming" gives an example matching my above
reply.  It says:

uniform BlockName3
{
  int mem;
} instanceName3[4];

This definition will create a single member named "BlockName3.min".
The reason this array of four blocks only counts as having one
variable is because each of the four blocks uses the same internal
definition. There is nothing that could be queried from
BlockName3[1] that could not be queried from BlockName3[0].

(*) 

Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Ian Romanick]

On 12/16/2016 09:35 PM, Kenneth Graunke wrote:
> This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
> arb_program_interface_query/arb_program_interface_query-resource-query,
> and GL45-CTS.program_interface_query.separate-programs-{tess-control,
> tess-eval,geometry}.  Only one dEQP program interface failure remains.
> 
> I would have liked to split this up into several distinct changes, but
> I wasn't sure how to do that given thet tangled nature of these issues.
> 
> So, the issues:
> 
>* We need to treat interface blocks declared as an array of instances
>  as a single block - removing the outer array.  The resource list
>  entry's name should not include the array length.  Properties such
>  as GL_ARRAY_SIZE should refer to the variable inside the block, not
>  the interface block's array properties.
> 
>* We need to do this prefixing even for structure variables.
> 
>* We need to do this for built-ins (such as gl_PerVertex.gl_Position).
> 
>* After interface array unwrapping, any variable which is an array
>  should have [0] appended.  It doesn't matter if it's a TCS/TES/GS
>  input or TCS output - that looked like an attempt to unwrap for
>  per-vertex variables, but that didn't consider per-patch variables,
>  and as far as I can tell there's nothing to justify this.
> 
> Several Mesa developers have suggested that Issue 16 contradicts the
> main specification, but I believe that it doesn't - the main spec just
> isn't terribly clear.  The main ARB_program_interface query spec says:
> 
>   "* For an active interface block not declared as an array of block
>  instances, a single entry will be generated, using the block name from
>  the shader source.
> 
>* For an active interface block declared as an array of instances,
>  separate entries will be generated for each active instance.  The name
>  of the instance is formed by concatenating the block name, the "["
>  character, an integer identifying the instance number, and the "]"
>  character."
> 
> Issue 16 says that built-ins should be named "gl_PerVertex.gl_Position",
> but several people suggested the second bullet above means that it
> should be named "gl_PerVertex[array length].gl_Position".
> 
> There are two important things to note.  Those bullet points say
> "an active interface block", while the others say "variable" or "active
> shader storage block member".  They also don't mention applying the
> rules recursively (unlike the other bullets).  Both suggest that
> these rules apply to blocks themselves, not members of blocks.
> 
> In fact, for GL_UNIFORM_BLOCK queries, we do have "block[0]",
> "block[1]", ... resource list entries - so those rules are real,
> and actually used.  So if they don't apply to block members, then how
> should members be named?  Unfortunately, I don't see any rules outside
> of issue 16 - where the rationale is very unclear.  I hope to clarify
> the spec in the future.

Based on my understanding, something like

out Vertex {
vec4 p;
vec4 c[3];
} vertex[2];

in a vertex shader should produce

Vertex[0].p
Vertex[0].c[0] (with GL_ARRAY_SIZE = 3)
Vertex[1].p
Vertex[1].c[0] (with GL_ARRAY_SIZE = 3)

This is definitely what we would produce for a uniform block.

What I've never understood is why that isn't done for gl_PerVertex stage
boundaries where gl_PerVertex is explicitly arrayed.

I think this patch is good for now.  I like that it brings all the
strange edge-case handling into one place.

Reviewed-by: Ian Romanick 

> Cc: Ian Romanick 
> Cc: Alejandro Piñeiro 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/compiler/glsl/linker.cpp   | 100 
> -
>  src/mesa/main/shader_query.cpp |  92 -
>  2 files changed, 79 insertions(+), 113 deletions(-)
> 
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 5066014..5508d58 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -3735,6 +3735,65 @@ add_shader_variable(const struct gl_context *ctx,
>  bool use_implicit_location, int location,
>  const glsl_type *outermost_struct_type = NULL)
>  {
> +   const glsl_type *interface_type = var->get_interface_type();
> +
> +   if (outermost_struct_type == NULL) {
> +  /* Unsized (non-patch) TCS output/TES input arrays are implicitly
> +   * sized to gl_MaxPatchVertices.  Internally, we shrink them to a
> +   * smaller size.
> +   *
> +   * This can cause trouble with SSO programs.  Since the TCS declares
> +   * the number of output vertices, we can always shrink TCS output
> +   * arrays.  However, the TES might not be linked with a TCS, in
> +   * which case it won't know the size of the patch.  In other words,
> +   

Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Alejandro Piñeiro]

On 17/12/16 17:57, Kenneth Graunke wrote:
> On Saturday, December 17, 2016 5:41:35 PM PST Alejandro Piñeiro wrote:
>> On 17/12/16 03:35, Kenneth Graunke wrote:
>>> This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
>>> arb_program_interface_query/arb_program_interface_query-resource-query,
>> Somewhat offtopic: FWIW, piglit tests for arb_program_interface_query
>> doesn't tests too much the REFERENCED_BY_XXX property (see below). I
>> have a wip piglit tests locally that still fails. Mentioning this
>> because ...
>>
>>> and GL45-CTS.program_interface_query.separate-programs-{tess-control,
>>> tess-eval,geometry}.  Only one dEQP program interface failure remains.
>> ... as far as I see, just one CTS program interface query keeps failing
>> after your patch, and it is caused by some issues with
>> REFERENCED_BY_XXX. In any case, about this, now Im somewhat more
>> optimistic, after realizing (while testing your patch) that Ian Romanick
>> fixed GL45-CTS.geometry_shader.program_resource.program_resource, and
>> cleaned up referenced_by on the patch "linker: Accurately track
>> gl_uniform_block::stageref" (commit 084105). Will take a look to that
>> next Monday.
> Is the one remaining failure
>
> GL45-CTS.program_interface_query.uniform-block-types
>
> ...?  

Yes that one.

> I believe that one's fixed by Ian's series:
>
> https://lists.freedesktop.org/archives/mesa-dev/2016-December/138359.html

Oh neat! Yes, that series fixes that test, just tested.

Thanks!



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Alejandro Piñeiro]

On 17/12/16 17:57, Kenneth Graunke wrote:
> On Saturday, December 17, 2016 5:41:35 PM PST Alejandro Piñeiro wrote:
>> On 17/12/16 03:35, Kenneth Graunke wrote:
>>> This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
>>> arb_program_interface_query/arb_program_interface_query-resource-query,
>> Somewhat offtopic: FWIW, piglit tests for arb_program_interface_query
>> doesn't tests too much the REFERENCED_BY_XXX property (see below). I
>> have a wip piglit tests locally that still fails. Mentioning this
>> because ...
>>
>>> and GL45-CTS.program_interface_query.separate-programs-{tess-control,
>>> tess-eval,geometry}.  Only one dEQP program interface failure remains.
>> ... as far as I see, just one CTS program interface query keeps failing
>> after your patch, and it is caused by some issues with
>> REFERENCED_BY_XXX. In any case, about this, now Im somewhat more
>> optimistic, after realizing (while testing your patch) that Ian Romanick
>> fixed GL45-CTS.geometry_shader.program_resource.program_resource, and
>> cleaned up referenced_by on the patch "linker: Accurately track
>> gl_uniform_block::stageref" (commit 084105). Will take a look to that
>> next Monday.
> Is the one remaining failure
>
> GL45-CTS.program_interface_query.uniform-block-types

Yes I meant that test.

>
> ...?  I believe that one's fixed by Ian's series:
>
> https://lists.freedesktop.org/archives/mesa-dev/2016-December/138359.html

Oh neat, just made a quick test, and yes it fixes that test.

BR




signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Kenneth Graunke]

On Saturday, December 17, 2016 5:41:35 PM PST Alejandro Piñeiro wrote:
> On 17/12/16 03:35, Kenneth Graunke wrote:
> > This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
> > arb_program_interface_query/arb_program_interface_query-resource-query,
> 
> Somewhat offtopic: FWIW, piglit tests for arb_program_interface_query
> doesn't tests too much the REFERENCED_BY_XXX property (see below). I
> have a wip piglit tests locally that still fails. Mentioning this
> because ...
> 
> > and GL45-CTS.program_interface_query.separate-programs-{tess-control,
> > tess-eval,geometry}.  Only one dEQP program interface failure remains.
> 
> ... as far as I see, just one CTS program interface query keeps failing
> after your patch, and it is caused by some issues with
> REFERENCED_BY_XXX. In any case, about this, now Im somewhat more
> optimistic, after realizing (while testing your patch) that Ian Romanick
> fixed GL45-CTS.geometry_shader.program_resource.program_resource, and
> cleaned up referenced_by on the patch "linker: Accurately track
> gl_uniform_block::stageref" (commit 084105). Will take a look to that
> next Monday.

Is the one remaining failure

GL45-CTS.program_interface_query.uniform-block-types

...?  I believe that one's fixed by Ian's series:

https://lists.freedesktop.org/archives/mesa-dev/2016-December/138359.html


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Alejandro Piñeiro]

On 17/12/16 03:35, Kenneth Graunke wrote:
> This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
> arb_program_interface_query/arb_program_interface_query-resource-query,

Somewhat offtopic: FWIW, piglit tests for arb_program_interface_query
doesn't tests too much the REFERENCED_BY_XXX property (see below). I
have a wip piglit tests locally that still fails. Mentioning this
because ...

> and GL45-CTS.program_interface_query.separate-programs-{tess-control,
> tess-eval,geometry}.  Only one dEQP program interface failure remains.

... as far as I see, just one CTS program interface query keeps failing
after your patch, and it is caused by some issues with
REFERENCED_BY_XXX. In any case, about this, now Im somewhat more
optimistic, after realizing (while testing your patch) that Ian Romanick
fixed GL45-CTS.geometry_shader.program_resource.program_resource, and
cleaned up referenced_by on the patch "linker: Accurately track
gl_uniform_block::stageref" (commit 084105). Will take a look to that
next Monday.

> I would have liked to split this up into several distinct changes, but
> I wasn't sure how to do that given thet tangled nature of these issues.
>
> So, the issues:
>
>* We need to treat interface blocks declared as an array of instances
>  as a single block - removing the outer array.  The resource list
>  entry's name should not include the array length.  Properties such
>  as GL_ARRAY_SIZE should refer to the variable inside the block, not
>  the interface block's array properties.
>
>* We need to do this prefixing even for structure variables.
>
>* We need to do this for built-ins (such as gl_PerVertex.gl_Position).
>
>* After interface array unwrapping, any variable which is an array
>  should have [0] appended.  It doesn't matter if it's a TCS/TES/GS
>  input or TCS output - that looked like an attempt to unwrap for
>  per-vertex variables, but that didn't consider per-patch variables,
>  and as far as I can tell there's nothing to justify this.
>
> Several Mesa developers have suggested that Issue 16 contradicts the
> main specification,

Me included ;), although probably I misunderstood the spec as ...

>  but I believe that it doesn't - the main spec just
> isn't terribly clear. 

... I agree.

>  The main ARB_program_interface query spec says:
>
>   "* For an active interface block not declared as an array of block
>  instances, a single entry will be generated, using the block name from
>  the shader source.
>
>* For an active interface block declared as an array of instances,
>  separate entries will be generated for each active instance.  The name
>  of the instance is formed by concatenating the block name, the "["
>  character, an integer identifying the instance number, and the "]"
>  character."
>
> Issue 16 says that built-ins should be named "gl_PerVertex.gl_Position",
> but several people suggested the second bullet above means that it
> should be named "gl_PerVertex[array length].gl_Position".
>
> There are two important things to note.  Those bullet points say
> "an active interface block", while the others say "variable" or "active
> shader storage block member".  They also don't mention applying the
> rules recursively (unlike the other bullets).  Both suggest that
> these rules apply to blocks themselves, not members of blocks.
>
> In fact, for GL_UNIFORM_BLOCK queries, we do have "block[0]",
> "block[1]", ... resource list entries - so those rules are real,
> and actually used.  So if they don't apply to block members, then how
> should members be named?  

Makes sense, although in a really convoluted way, as it seems very
interpretative. But that goes back to the lack of clearness of the spec.
Perhaps (I hope?) all this work would help to clarify it.

> Unfortunately, I don't see any rules outside
> of issue 16 - where the rationale is very unclear.  I hope to clarify
> the spec in the future.

Yes, I agree. Not sure what was the intention behind issue 16. Seems an
exception to the common rule, but not sure why it was needed.

>
> Cc: Ian Romanick 
> Cc: Alejandro Piñeiro 
> Signed-off-by: Kenneth Graunke 
> ---
>  src/compiler/glsl/linker.cpp   | 100 
> -
>  src/mesa/main/shader_query.cpp |  92 -
>  2 files changed, 79 insertions(+), 113 deletions(-)
>
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index 5066014..5508d58 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -3735,6 +3735,65 @@ add_shader_variable(const struct gl_context *ctx,
>  bool use_implicit_location, int location,
>  const glsl_type *outermost_struct_type = NULL)
>  {
> +   const glsl_type *interface_type = var->get_interface_type();
> +
> +   if (outermost_struct_type == 

[Mesa-dev] [PATCH] glsl: Fix program interface queries relating to interface blocks.

[Kenneth Graunke]

This fixes 555 dEQP tests (using the nougat-cts-dev branch), Piglit's
arb_program_interface_query/arb_program_interface_query-resource-query,
and GL45-CTS.program_interface_query.separate-programs-{tess-control,
tess-eval,geometry}.  Only one dEQP program interface failure remains.

I would have liked to split this up into several distinct changes, but
I wasn't sure how to do that given thet tangled nature of these issues.

So, the issues:

   * We need to treat interface blocks declared as an array of instances
 as a single block - removing the outer array.  The resource list
 entry's name should not include the array length.  Properties such
 as GL_ARRAY_SIZE should refer to the variable inside the block, not
 the interface block's array properties.

   * We need to do this prefixing even for structure variables.

   * We need to do this for built-ins (such as gl_PerVertex.gl_Position).

   * After interface array unwrapping, any variable which is an array
 should have [0] appended.  It doesn't matter if it's a TCS/TES/GS
 input or TCS output - that looked like an attempt to unwrap for
 per-vertex variables, but that didn't consider per-patch variables,
 and as far as I can tell there's nothing to justify this.

Several Mesa developers have suggested that Issue 16 contradicts the
main specification, but I believe that it doesn't - the main spec just
isn't terribly clear.  The main ARB_program_interface query spec says:

  "* For an active interface block not declared as an array of block
 instances, a single entry will be generated, using the block name from
 the shader source.

   * For an active interface block declared as an array of instances,
 separate entries will be generated for each active instance.  The name
 of the instance is formed by concatenating the block name, the "["
 character, an integer identifying the instance number, and the "]"
 character."

Issue 16 says that built-ins should be named "gl_PerVertex.gl_Position",
but several people suggested the second bullet above means that it
should be named "gl_PerVertex[array length].gl_Position".

There are two important things to note.  Those bullet points say
"an active interface block", while the others say "variable" or "active
shader storage block member".  They also don't mention applying the
rules recursively (unlike the other bullets).  Both suggest that
these rules apply to blocks themselves, not members of blocks.

In fact, for GL_UNIFORM_BLOCK queries, we do have "block[0]",
"block[1]", ... resource list entries - so those rules are real,
and actually used.  So if they don't apply to block members, then how
should members be named?  Unfortunately, I don't see any rules outside
of issue 16 - where the rationale is very unclear.  I hope to clarify
the spec in the future.

Cc: Ian Romanick 
Cc: Alejandro Piñeiro 
Signed-off-by: Kenneth Graunke 
---
 src/compiler/glsl/linker.cpp   | 100 -
 src/mesa/main/shader_query.cpp |  92 -
 2 files changed, 79 insertions(+), 113 deletions(-)

diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 5066014..5508d58 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -3735,6 +3735,65 @@ add_shader_variable(const struct gl_context *ctx,
 bool use_implicit_location, int location,
 const glsl_type *outermost_struct_type = NULL)
 {
+   const glsl_type *interface_type = var->get_interface_type();
+
+   if (outermost_struct_type == NULL) {
+  /* Unsized (non-patch) TCS output/TES input arrays are implicitly
+   * sized to gl_MaxPatchVertices.  Internally, we shrink them to a
+   * smaller size.
+   *
+   * This can cause trouble with SSO programs.  Since the TCS declares
+   * the number of output vertices, we can always shrink TCS output
+   * arrays.  However, the TES might not be linked with a TCS, in
+   * which case it won't know the size of the patch.  In other words,
+   * the TCS and TES may disagree on the (smaller) array sizes.  This
+   * can result in the resource names differing across stages, causing
+   * SSO validation failures and other cascading issues.
+   *
+   * Expanding the array size to the full gl_MaxPatchVertices fixes
+   * these issues.  It's also what program interface queries expect,
+   * as that is the official size of the array.
+   */
+  if (var->data.tess_varying_implicit_sized_array) {
+ type = resize_to_max_patch_vertices(ctx, type);
+ interface_type = resize_to_max_patch_vertices(ctx, interface_type);
+  }
+
+  if (var->data.from_named_ifc_block) {
+ const char *interface_name = interface_type->name;
+
+ if (interface_type->is_array()) {
+/* Issue #16 of the