[Mesa-dev] [PATCH 1/2] glsl: add driconf to zero-init unintialized vars

[Rob Clark]

On Mon, Jun 27, 2016 at 3:06 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Monday, June 27, 2016 11:43:28 AM PDT Matt Turner wrote:
>> On Mon, Jun 27, 2016 at 4:44 AM, Rob Clark <robdclark at gmail.com> wrote:
>> > On Mon, Jun 27, 2016 at 7:13 AM, Alan Swanson <reiver at improbability.net> wrote:
>> >> On 2016-06-25 13:37, Rob Clark wrote:
>> >>>
>> >>> Some games are sloppy.. perhaps because it is defined behavior for DX or
>> >>> perhaps because nv blob driver defaults things to zero.
>> >>>
>> >>> So add driconf param to force uninitialized variables to default to zero.
>> >>>
>> >>> This issue was observed with rust, from steam store.  But has surfaced
>> >>> elsewhere in the past.
>> >>>
>> >>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> >>> ---
>> >>> Note that I left out the drirc bit, since not entirely sure how to
>> >>> identify this game.  (I don't actually have the game, just working off
>> >>> of an apitrace)
>> >>>
>> >>> Possibly worth mentioning that for the shaders using uninitialized vars
>> >>> having zero-initializers lets constant-propagation get rid of a whole
>> >>> lot of instructions.  One shader I saw dropped to less than half of
>> >>> it's original instruction count.
>> >>
>> >>
>> >> If the default for uninitialised variables is undefined, then with the
>> >> reported shader optimisations why bother with the (DRI) option when
>> >> zeroing could still essentially be classed as undefined?
>> >>
>> >> Cuts the patch down to just the src/compiler/glsl/ast_to_hir.cpp change.
>> >
>> > I did suggest that on #dri-devel, but Jason had a theoretical example
>> > where it would hurt.. iirc something like:
>> >
>> >   float maybe_undef;
>> >   for (int i = 0; i < some_uniform_at_least_one; i++)
>> >      maybe_undef = ...
>> >
>> > also, he didn't want to hide shader bugs that app should fix.
>> >
>> > It would be interesting to rush shaderdb w/ glsl_zero_init=true and
>> > see what happens, but I didn't get around to that yet.
>>
>> Here's what I get on i965. It's not a clear win.
>>
>> total instructions in shared programs: 5249030 -> 5249002 (-0.00%)
>> instructions in affected programs: 28936 -> 28908 (-0.10%)
>> helped: 66
>> HURT: 132
>>
>> total cycles in shared programs: 57966694 -> 57956306 (-0.02%)
>> cycles in affected programs: 1136118 -> 1125730 (-0.91%)
>> helped: 78
>> HURT: 106
>
> I suspect most of the help is because we're missing undef optimizations,
> such as CSE...while zero could be CSE'd.  (I have a patch, but it hurts
> things too...)

right, I was thinking that treating undef as zero in constant-folding
would have the same effect.. ofc it might make shader bugs less
obvious.

Btw, does anyone know what fglrx does?  Afaiu nv blob treats undef as
zero.  If fglrx does the same, I suppose that strengthens the argument
for "just do this unconditionally".

(but I'm still leaning towards "make it conditional" so far..)

BR,
-R