[Mesa-dev] [PATCH 00/23] Handle Booleans and things better

[Ian Romanick]

On 03/20/2015 03:13 PM, Matt Turner wrote:
> On Fri, Mar 20, 2015 at 1:58 PM, Ian Romanick <idr at freedesktop.org> wrote:
>> This is a series that I've been working on for quite some time.  Some of
>> these patches data from as long ago as July 2014.  It's finally time to
>> just send it out.  I have felt like I have been shaving an infinite yak,
>> and I have some additional patchs in-progress that will go on top of
>> this series.
>>
>> Overall results for the without NIR:
>>
>> GM45 (0x2A42):
>> total instructions in shared programs: 3550788 -> 3544141 (-0.19%)
>> instructions in affected programs:     513721 -> 507074 (-1.29%)
>> helped:                                1409
>> HURT:                                  269
>>
>> Iron Lake (0x0046):
>> total instructions in shared programs: 4980593 -> 4973020 (-0.15%)
>> instructions in affected programs:     572150 -> 564577 (-1.32%)
>> helped:                                1649
>> HURT:                                  386
>> GAINED:                                5
>> LOST:                                  24
>>
>> Sandy Bridge (0x0116):
>> total instructions in shared programs: 6817068 -> 6797680 (-0.28%)
>> instructions in affected programs:     1388229 -> 1368841 (-1.40%)
>> helped:                                5343
>> HURT:                                  319
>> GAINED:                                4
>> LOST:                                  11
>>
>> Ivy Bridge (0x0166):
>> total instructions in shared programs: 6290138 -> 6273022 (-0.27%)
>> instructions in affected programs:     1204828 -> 1187712 (-1.42%)
>> helped:                                4558
>> HURT:                                  825
>> GAINED:                                1
>> LOST:                                  3
>>
>> Haswell (0x0426):
>> total instructions in shared programs: 5777876 -> 5759525 (-0.32%)
>> instructions in affected programs:     1130638 -> 1112287 (-1.62%)
>> helped:                                4830
>> HURT:                                  606
>> GAINED:                                1
>> LOST:                                  3
>>
>> Broadwell (0x162E):
>> total instructions in shared programs: 6821191 -> 6806526 (-0.21%)
>> instructions in affected programs:     1252081 -> 1237416 (-1.17%)
>> helped:                                5090
>> HURT:                                  608
>> GAINED:                                4
>> LOST:                                  4
>>
>> In some ways the results with NIR are even better (if you ignore Sandy
>> Bridge):
>>
>> Sandy Bridge (0x0116) NIR:
>> total instructions in shared programs: 6820689 -> 6785869 (-0.51%)
>> instructions in affected programs:     1858575 -> 1823755 (-1.87%)
>> helped:                                4559
>> HURT:                                  3320
>> GAINED:                                3
>> LOST:                                  29
>>
>> Ivy Bridge (0x0166) NIR:
>> total instructions in shared programs: 6309309 -> 6276897 (-0.51%)
>> instructions in affected programs:     1639688 -> 1607276 (-1.98%)
>> helped:                                4053
>> HURT:                                  3496
>> GAINED:                                34
>> LOST:                                  3
>>
>> Haswell (0x0426) NIR:
>> total instructions in shared programs: 5790261 -> 5751689 (-0.67%)
>> instructions in affected programs:     1506127 -> 1467555 (-2.56%)
>> helped:                                5407
>> HURT:                                  1957
>> GAINED:                                28
>> LOST:                                  3
>>
>> Broadwell (0x162E) NIR:
>> total instructions in shared programs: 7012929 -> 6977716 (-0.50%)
>> instructions in affected programs:     1604094 -> 1568881 (-2.20%)
>> helped:                                5626
>> HURT:                                  2029
>> GAINED:                                20
>> LOST:                                  3
>>
>> In that there are 12 shaders that are hurt (by ~40 instructions) because
>> we are now able to unroll a loop.  This is noted in the commit message
>> of "glsl: Optimize certain if-statements to just casts from the
>> condition".
>>
>> There are a couple commits that hurt a fair number of shaders with NIR.
>> It doesn't seem like this should be the case, and it seems like there is
>> probably work that can be done in brw_fs_nir.cpp to account for this.
>> Without some of the initial changes to brw_fs_visitor.cpp, there were
>> also regressions without NIR.  All of the changes in this series at the
>> GLSL IR level are things that could have been written in the shader
>> source.
>>
>> At the end of the day on Broadwell with NIR, there are a bunch of
>> shaders (some as small as ~22 instructions) hurt by 1 to 4 instructions.
>> Only one shader is really hurt (119 -> 139).  This shader is noted in
>> patch "glsl: Distribute multiply over b2f".  At the same time, over 250
>> shaders with more than 100 instructions are helped by more than 20%.
>> Oh, and we gained 20 SIMD16 shaders. :)
>>
>> This series is also available as the bool-optimizations-v6 branch in my
>> fd.o Mesa repo.
> 
> So, I want to review this, but I don't know how.
> 
> You've Cc'd various people on various patches. I guess I shouldn't review those?
> 
> But there are some patches that don't have anyone Cc'd. Can I review those?
> 
> This feels really stupid reading through a bunch of patches to find
> ones that don't have people Cc'd that I can review, while still having
> to read the others to get the context of the patch I'm reviewing.
> 
> Should I just ignore the Ccs and review them all?

Erk.  I was trying to get some people who don't have many reviews to...
get some reviews by sending .  I hadn't considered that doing so would
make it more difficult / annoying to review the other patches.  I think
getting code reviewed is more important than getting someone's numbers
up, so do whatever works best for you.

The only patch that I really want the Cc'ed person to review patch 18,
but I wouldn't ignore comments from others on that one.