[Mesa-dev] [PATCH] i965: Use vec4 vs shader path in spill cases.

[Bish, Jim]

On 06/04/2015 04:35 PM, Ben Widawsky wrote:
> On Wed, Jun 03, 2015 at 05:42:10PM -0700, Matt Turner wrote:
>> On Wed, Jun 3, 2015 at 5:29 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
>>> On Wednesday, June 03, 2015 11:35:43 PM Bish, Jim wrote:
>>>> For BDW and newer hardware, vertex shaders can be dispatched
>>>> using simd8 scalars, but when spills occur performance will
>>>> be impacted.
>>>>
>>>> Fixes spec at glsl-1.10@execution at varying-packing@simple by falling back
>>>> to vec4x2 dispatch. There is another issue with simd8 dispatch which
>>>> will be addressed later.
>>>>
>>>> Bugzill : https://bugs.freedesktop.org/show_bug.cgi?id=89055
>>>
>>> Hi Jim,
>>>
>>> That link appears to be a Skylake display bug - presumably you meant
>>> this one?
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89786
>>>
>>> Based on Neil's comments, it sounds like there's a bug in the SIMD8
>>> backend's register spilling code, which is worth fixing.  This seems
>>> to paper over that by not using SIMD8 mode.  I'd rather not do that.
ok.  Yes there is a real issue with simd8 dispatch on VS and perhaps on GS, FS. 
First the spills are really bad and does need to be fix.  for the varying-packing-simple
referenced in the bug.  The first issue is what I was addressing in the patch
here is the dump of simd8

VS8 estimated execution time: 2130 cycles
Native code for unnamed vertex shader 3
SIMD8 shader: 518 instructions. 0 loops. 45:45 spills:fills. Promoted 0 constants. Compacted 8288 to 5008 bytes (40%)


versus the dump from vec4

VS estimated execution time: 600 cycles
Native code for unnamed vertex shader 3:
VS vec4 shader: 288 instructions. 0 loops. Compacted 4608 to 3072 bytes (33%)

very bad on the spills.  The second issue here is that when we are under 
significant register pressure, we map a virt register to phys at 123 and add
offset to the register which spills into the MRF hack registers like this

mov(8)          g126<1>F        g114<8,8,1>D                    { align1 1Q compacted };
mov(8)          g106<1>F        g2<0,1,0>D                      { align1 1Q compacted };
mov(8)          g127<1>UD       g106<8,8,1>UD                   { align1 1Q compacted };
mov(8)          g126<1>UD       g0<8,8,1>UD                     { align1 WE_all 1Q compacted };
mov(1)          g126.2<1>UD     0x0000000aUD                    { align1 WE_all compacted };
send(8)         null            g126<8,8,1>UD

my original plan was to fix issue 1 and 2 but then decided to do it in
two steps. 

>>>
>>> I'm happy to consider falling back to SIMD4x2 when spilling, for better
>>> performance - it certainly seems reasonable - but we should obtain some
>>> performance data showing that an application benefits from that change.
>>
>> Indeed, this is something we've talked about before, but it's never
>> been clear that it would be a win. There are also some unsolved
>> problems with that approach, like Ken mentioned.

ok will collect some data on this and compare.  I also believe there
are some issues with spilling in general that are hurting performance but will
get to that later. 
>>
>> I ran the patch through our Jenkins CI system and while it does fix 26
>> tests on BDW, it causes an assertion failure in
>> tests/spec/glsl-1.30/execution/varying-packing-mixed-types.shader_test:
>>
>> shader_runner: ../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp:1071:
>> virtual void brw::vec4_visitor::visit(ir_variable*): Assertion
>> `this->uniforms < uniform_array_size' failed.
whoops - will run the shader_db tests as well and fix. I only ran piglit
my bad.
>>
>> I ran the patch through shader-db, and there were no changes, but
>> that's because we have no vertex shaders that spill. Until we have
>> vertex shaders that spill, I don't think figuring out how to fall back
>> to vec4 and generate reasonable code is particularly compelling.

what platform?  this code only kicks in on BDW and SKL systems?  did 
you look at the piglit test referenced in teh bug?
> 
> What about geometry shaders when Ken gets those hooked up?

yes would happen here too. 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>