[Mesa-dev] [PATCH 0/6] Eliminating unused built-in varyings

[Ian Romanick]

On 06/28/2013 10:55 AM, Marek Olšák wrote:
> On Fri, Jun 28, 2013 at 5:42 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> On 06/13/2013 05:25 AM, Marek Olšák wrote:
>>>
>>> Hi everyone,
>>>
>>> this series adds a new GLSL compiler optimization pass which eliminates
>>> unused and set-but-unused built-in varyings and adds a few improvements to
>>> the GLSL linker in the process.
>>>
>>> Before I show you how it works, I wanna say that there are patches which
>>> are related to and will most probably conflict with the geometry shader
>>> work, but they are necessary because the linkage of varyings is largely
>>> suboptimal.
>>>
>>> Also, the GL_EXT_separate_shader_objects extension must be disabled
>>> for this optimization to be enabled. The reason is a program object with
>>> both a VS and FS can be bound partially, e.g. by
>>> glUseShaderProgramEXT(GL_VERTEX_SHADER, prog), so the extension makes
>>> every program object be just a set of "separate shaders". The extension
>>> is not important anyway.
>>
>> Could you elaborate on this a bit?  The problem already exists that shader
>> can be par-linked (e.g., just a vertex shader) and used with fixed-function.
>> A big part of the reason that I implemented EXT_sso back in the day was to
>> generate infrastructure, etc. for better using shaders with fixed function.
>
> The only problem I have with EXT_sso is that you can link two
> programs, both having a vertex and fragment shader, and still
> mix-and-match their shaders independently as if all the shaders were
> separate/unlinked. For example:

Oh, I had forgotten all about that.  I remember thinking the spec 
authors must have been drunk when they didn't add language to prevent 
that usage.  That makes perfect sense, then.  It sounds like I should 
get off my butt, land Gergory Hainaut's ARB_sso work, and gut out EXT_sso.

> /* equivalent to glUseProgram(prog1); */
> glUseShaderProgramEXT(GL_VERTEX_SHADER, prog1);
> glUseShaderProgramEXT(GL_FRAGMENT_SHADER, prog1);
>
> /* prog1 has its own fragment shader, but who cares! we don't have to bind it */
> /* prog2 also has its own vertex shader */
> glUseShaderProgramEXT(GL_VERTEX_SHADER, prog1);
> glUseShaderProgramEXT(GL_FRAGMENT_SHADER, prog2);
>
> /* more fun, we can put a geometry shader in between */
> glUseShaderProgramEXT(GL_VERTEX_SHADER, prog1);
> glUseShaderProgramEXT(GL_GEOMETRY_SHADER, prog3);
> glUseShaderProgramEXT(GL_FRAGMENT_SHADER, prog1);
>
> /* assume prog3 has all 3 shaders, we can unbind the middle one */
> glUseProgram(prog3);
> glUseShaderProgramEXT(GL_GEOMETRY_SHADER, NULL);
>
> I have no problem with linking program objects with a single shader
> and mixing and matching such program objects. However the ability to
> mix-and-match shaders no matter what program object they are part of
> is a real show-stopper for this optimization. Thankfully, this doesn't
> happen with fixed function and ARB_sso also forbids such usage.
>
>>> Now, to illustrate how the optimization works, consider these 2 shader IR
>>> dumps:
>>>
>>>
>>> Vertex shader (8 varyings):
>>> ...
>>> (declare (shader_out ) vec4 gl_FrontColor)
>>> (declare (shader_out ) vec4 gl_FrontSecondaryColor)
>>> (declare (shader_out ) (array vec4 6) gl_TexCoord)
>>> (function main
>>>     (signature void
>>>       (parameters
>>>       )
>>>       (
>>>         ...
>>>         (assign  (xyzw) (var_ref gl_FrontColor)  (var_ref gl_Color) )
>>>         (assign  (xyzw) (var_ref gl_FrontSecondaryColor)  (var_ref
>>> gl_SecondaryColor) )
>>>         (assign  (xyzw) (array_ref (var_ref gl_TexCoord) (constant int (1))
>>> )  (var_ref gl_MultiTexCoord1) )
>>>         (assign  (xyzw) (array_ref (var_ref gl_TexCoord) (constant int (4))
>>> )  (var_ref gl_MultiTexCoord4) )
>>>         (assign  (xyzw) (array_ref (var_ref gl_TexCoord) (constant int (5))
>>> )  (var_ref gl_MultiTexCoord5) )
>>>       ))
>>> )
>>>
>>> Fragment shader (6 varyings):
>>> ...
>>> (declare (shader_in ) vec4 gl_SecondaryColor)
>>> (declare (shader_in ) (array vec4 5) gl_TexCoord)
>>> (function main
>>>     (signature void
>>>       (parameters
>>>       )
>>>       (
>>>         (declare () vec4 r)
>>>         (assign  (xyzw) (var_ref r)  ... (var_ref gl_SecondaryColor) ) )
>>>         (assign  (xyzw) (var_ref r)  ... (array_ref (var_ref gl_TexCoord)
>>> (constant int (1)) ) ) ) )
>>>         (assign  (xyzw) (var_ref r)  ... (array_ref (var_ref gl_TexCoord)
>>> (constant int (2)) ) ) ) )
>>>         (assign  (xyzw) (var_ref r)  ... (array_ref (var_ref gl_TexCoord)
>>> (constant int (3)) ) ) ) )
>>>         (declare (temporary ) vec4 assignment_tmp)
>>>         (assign  (xyzw) (var_ref assignment_tmp)  ... (array_ref (var_ref
>>> gl_TexCoord) (constant int (4)) ) ) ) )
>>>         ...
>>>       ))
>>> )
>>>
>>>
>>> Note that only gl_TexCoord[1], gl_TexCoord[4], and gl_SecondaryColor
>>> are used by both shaders. The optimization replaces all occurences of
>>> varyings which are unused by the other stage by temporary variables. It
>>> also breaks down the gl_TexCoord array into separate vec4 variables if
>>> needed. Here's the result:
>>
>> This sounds similar to the way Paul's varying packing works.  Is there
>> synergy there?  Also, since variables are renamed, does this interact with
>> transform feedback?  The queries of GL_ARB_program_interface_query?  I
>> suspect that it won't since this only affects built-in varyings, and
>> built-in varyings aren't usable with those interfaces.
>
> I haven't followed the varying packing work really, so I'm not sure,
> but it seemed to be only about user-defined varyings, while my work is
> only about built-in varyings.

That is correct.  Both, however, work by generating some new names and 
putting existing things in them.  At some point we might want to see if 
some code could get shared between them.

> Concerning transform feedback, the optimization pass receives the list
> of TFB varyings and doesn't eliminate them.

Yeah, I wrote the first message before looking at the patches.  It looks 
like you handled it in a reasonable way.

> Concerning GL_ARB_program_interface_query, there doesn't seem to be
> any interaction with my work, like you say.
>
> Marek

I had a couple comments on patch 4, but patches 1, 2, 3, and 6, as-is, are

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>