[Mesa-dev] [PATCH 04/13] nir: Collapse more repeated bcsels on the same argument

[Ian Romanick]

On 08/02/2018 05:52 PM, Timothy Arceri wrote:
> I thought you were trying to move these type of opts to your new pass?
> 
> Or should I be splitting out and resending some of the bcsel opts in my
> dirt showdown patch [1]? And trying again to get the other patches from
> the series reviewed.

The new pass recognizes bcsel with all three arguments of Boolean as if
it were (src0 && src1) || (!src0 && src2).  It also recognizes
nir_instr_type_load_const of 0 or ~0 as Boolean.  See
https://cgit.freedesktop.org/~idr/mesa/commit/?h=opt-minimize-Boolean&id=1f2eea49f09b68ca6575fa18506520bc03930af8.
 When I ran the Dirt Showdown shader through the code in the branch, it
got massively reduced.

In the shaders that were helped by this pattern, b, c, and d were not
Boolean.  They were results of float calculations.

> [1] https://lists.freedesktop.org/archives/mesa-dev/2018-July/200510.html

Looking at the patch, the only patterns that my opt-minimize-Boolean
pass might not cover are:

   (('bcsel', ('fge', a, b), c, ('flt', a, b)), ('bcsel', ('fge', a, b),
c, True)),
   (('bcsel', ('fge', a, b), ('ior', ('flt', a, b), c), d), ('bcsel',
('fge', a, b), c, d)),

Do these patterns help by themselves?  If so, I don't see a reason to
not land them.  Looking at my branch from before I started
opt-minimize-Boolean, I had these and a couple others.  See
https://cgit.freedesktop.org/~idr/mesa/commit/?h=experiments&id=eab56383007a36acc3b60f154d402c9daf5b39c1.
 I think the second pattern should be marked a imprecise, and there's a
comment in my patch explaining.  I think all the rest of the patterns in
my patch are handled by opt-minimize-Boolean.

I have as future work to detect that a<b is the inverse of a>=b so that
the truth table evaluation can treat both expressions as a single input
to the table.  I also have as future work to detect that a<b is the
inverse of a>=b for the purposes of CSE.  It may be that doing the CSE
thing makes the other thing (and a handful of algebraic patterns)
unnecessary.

So far, this branch helps a lot of shaders.  Some, like the Dirt
Showdown shader, are helped a lot.  I haven't sent it out for review yet
because there are still a bunch of shaders that are hurt.  Some are hurt
a lot... many of the hurt shaders are hurt because they get more spills.
:(  It's getting closer, but I don't have a firm ETA.

> On 03/08/18 04:19, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> All Gen platforms had pretty similar results. (Skylake shown)
>> total instructions in shared programs: 14277230 -> 14277220 (<.01%)
>> instructions in affected programs: 751 -> 741 (-1.33%)
>> helped: 4
>> HURT: 0
>> helped stats (abs) min: 2 max: 3 x̄: 2.50 x̃: 2
>> helped stats (rel) min: 1.23% max: 1.40% x̄: 1.32% x̃: 1.32%
>> 95% mean confidence interval for instructions value: -3.42 -1.58
>> 95% mean confidence interval for instructions %-change: -1.47% -1.17%
>> Instructions are helped.
>>
>> total cycles in shared programs: 532577947 -> 532577908 (<.01%)
>> cycles in affected programs: 10641 -> 10602 (-0.37%)
>> helped: 4
>> HURT: 3
>> helped stats (abs) min: 1 max: 40 x̄: 13.75 x̃: 7
>> helped stats (rel) min: 0.11% max: 3.08% x̄: 1.10% x̃: 0.60%
>> HURT stats (abs)   min: 2 max: 8 x̄: 5.33 x̃: 6
>> HURT stats (rel)   min: 0.13% max: 0.55% x̄: 0.30% x̃: 0.23%
>> 95% mean confidence interval for cycles value: -20.69 9.55
>> 95% mean confidence interval for cycles %-change: -1.63% 0.63%
>> Inconclusive result (value mean confidence interval includes 0).
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>>   src/compiler/nir/nir_opt_algebraic.py | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/compiler/nir/nir_opt_algebraic.py
>> b/src/compiler/nir/nir_opt_algebraic.py
>> index fdd1af9d177..388c6354ff0 100644
>> --- a/src/compiler/nir/nir_opt_algebraic.py
>> +++ b/src/compiler/nir/nir_opt_algebraic.py
>> @@ -235,6 +235,7 @@ optimizations = [
>>      (('~bcsel', ('fge', b, a), b, a), ('fmax', a, b)),
>>      (('bcsel', ('inot', a), b, c), ('bcsel', a, c, b)),
>>      (('bcsel', a, ('bcsel', a, b, c), d), ('bcsel', a, b, d)),
>> +   (('bcsel', a, b, ('bcsel', a, c, d)), ('bcsel', a, b, d)),
>>      (('bcsel', a, True, 'b at bool'), ('ior', a, b)),
>>      (('fmin', a, a), a),
>>      (('fmax', a, a), a),