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

[Rob Clark]

On Wed, Jun 29, 2016 at 12:43 AM, Eirik Byrkjeflot Anonsen
<eirik at eirikba.org> wrote:
> Rob Clark <robdclark at gmail.com> writes:
>
>> On Tue, Jun 28, 2016 at 11:28 AM, Marek Olšák <maraeo at gmail.com> wrote:
>>> On Mon, Jun 27, 2016 at 9:28 PM, Rob Clark <robdclark at gmail.com> wrote:
>>>> 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".
>>>
>>> No idea what fglrx does, but LLVM does eliminate code with undefined
>>> inputs. Initializing everything to 0 might make that worse.
>>
>> hmm, treating as zero does eliminate a lot.. anyway, I guess we'll
>> stick w/ driconf.
>>
>> fwiw, with some help from the reporter, we figured out that this is
>> the bit that I need to squash into drirc:
>>
>>     <application name="Rust" executable="rust">
>>             <option name="glsl_zero_init value="true"/>
>>     </application>
>
> Not knowing a lot about drirc, I suspect you should have a double quote
> at the end of glsl_zero_init as well?

yup, that was a typo

BR,
-R

> eirik
>
>> now, if I could talk somebody into a r-b for this and the i965 fix? ;-)
>>
>> BR,
>> -R
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev