[Mesa-dev] [PATCH 2/2] mesa: add hard limits for the number of varyings and uniforms for the linker

[Marek Olšák]

On Wed, Nov 23, 2011 at 7:27 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 11/23/2011 05:42 AM, Marek Olšák wrote:
>>
>> On Wed, Nov 23, 2011 at 6:59 AM, Kenneth Graunke<kenneth at whitecape.org>
>>  wrote:
>>>
>>> So, again, if the interest is in making more apps succeed, we should
>>> start with varying packing.  That's useful all around, and I doubt
>>> anyone can dispute that it's necessary.
>>
>> No, that's not the problem. Varying packing is indeed important, but
>> it's far less important that the problem this discussion is all about.
>> We should start with breaking arrays into elements when possible when
>> doing those checks. Consider this shader:
>>
>> varying vec4 array[5];
>> ...
>> gl_TexCoord[7] = ...; /* adds 8*4 varyings components */
>> array[4] = ...; /* adds 5*4 varying components */
>>
>> /* linker stats: 13*4 varying components used -->  FAIL */
>> /* r300g stats: 2*4 components used ->  PASS */
>
> Do you have any concrete, real-world examples of this?  If so, can you name
> the applications?  I'd like to add tests that model their behavior to
> piglit.

Civilization 4 in Wine. I think that's the kind of an app people would
like to use Wine for.

https://bugs.freedesktop.org/show_bug.cgi?id=43121

>
> Every example that I have seen has looked more like:
>
> varying vec2 array[4];  // should be 8 varying components
>
> gl_TexCoord[2] = ...;
> gl_TexCoord[5] = ...;   // should be at most 24 varying components
>
> for (i = 0; i < array.length(); i++)
>    array[i] = ...;
>
> In this case, array gets stored as 4 vec4s using 16 varying components.  As
> a result, 16+24 doesn't fit when 8+24 would have.
>
>> It's the exact same problem with uniforms. The thing is r300g has
>> these optimizations already implemented for varyings and for uniforms.
>> Disabling not just one, but two optimizations at the same time just
>> because they should be done in the GLSL compiler and not in the
>> driver, seems quite unfriendly to me, almost like you didn't want me
>> to enable them. I would probably implement them in the GLSL compiler,
>
> That's overstating things quite a bit.  The optimizations aren't disabled.
>  I'm sure that r300g still gets significant performance benefits from this,
> and nothing has changed that.  The resource counting has always existed, so
> nothing has changed there either.

The elimination of unused uniforms gives no performance benefits
(there is a even slight performance penalty due to a remapping table
being used). The only motivation to write it was that the driver could
run more Wine apps and also we were able to close some bugs with it (I
guess they can be pretty much re-opened now, great). The elimination
of unused varyings is a simple dead-code elimination and the outputs
are packed next to each other because that's how hardware expects them
to be.

Also, the color varyings shouldn't contribute to the number of used
varying components on some (or all?) GPUs, because there are dedicated
outputs for them. I just hope you don't add the back colors too.

Now back on topic. It's now clear to me that if any shader can't run,
you want it to fail linking. However the r300 compiler may still fail
in a number of cases, even now. Do you want to report a failure every
time? If yes, then we need an interface for drivers to be able to
report compile errors in a form that can be presented to the user, and
at the same time the driver should be the one to decide whether a
shader can be run, not some shared component. Do you have any
opinions?

Marek