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

[Bish, Jim]

On 06/03/2015 05:29 PM, Kenneth Graunke 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

yes will update with better fix and more testing.  cut and paste
error - sorry
> 
> 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.
> 
> 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.
> 
> As is, I don't think this quite does what you want.
> 
> Earlier, process_glsl_ir() assumed this was a scalar shader stage, and
> called brw_do_channel_expressions and brw_do_vector_splitting to
> (destructively) split all vector operations into scalar operations in
> the high level IR.
> 
> Calling the vec4 backend with scalarized IR is likely to generate
> terribly inefficient code.  Ideally, we'd avoid scalarizing somehow...
> 
> --Ken
> 
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 34 +++++++++++++++++++++-------------
>>  1 file changed, 21 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> index 5a9c3f5..6ea9f92 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> @@ -1909,26 +1909,34 @@ brw_vs_emit(struct brw_context *brw,
>>  
>>           return NULL;
>>        }
>> -
>> -      fs_generator g(brw, mem_ctx, (void *) &c->key, &prog_data->base.base,
>> +      
>> +      if (!v.spilled_any_registers) {
>> +         fs_generator g(brw, mem_ctx, (void *) &c->key, &prog_data->base.base,
>>                       &c->vp->program.Base, v.promoted_constants,
>>                       v.runtime_check_aads_emit, "VS");
>> -      if (INTEL_DEBUG & DEBUG_VS) {
>> -         char *name;
>> -         if (prog) {
>> -            name = ralloc_asprintf(mem_ctx, "%s vertex shader %d",
>> +         if (INTEL_DEBUG & DEBUG_VS) {
>> +            char *name;
>> +            if (prog) {
>> +               name = ralloc_asprintf(mem_ctx, "%s vertex shader %d",
>>                                     prog->Label ? prog->Label : "unnamed",
>>                                     prog->Name);
>> -         } else {
>> -            name = ralloc_asprintf(mem_ctx, "vertex program %d",
>> +            } else {
>> +               name = ralloc_asprintf(mem_ctx, "vertex program %d",
>>                                     c->vp->program.Base.Id);
>> +            }
>> +            g.enable_debug(name);
>>           }
>> -         g.enable_debug(name);
>> -      }
>> -      g.generate_code(v.cfg, 8);
>> -      assembly = g.get_assembly(final_assembly_size);
>> +         g.generate_code(v.cfg, 8);
>> +         assembly = g.get_assembly(final_assembly_size);
>>  
>> -      c->base.last_scratch = v.last_scratch;
>> +         c->base.last_scratch = v.last_scratch;
>> +      } else {
>> +         /*
>> +          * Clean up from previous attempt to use simd8 dispatch
>> +          */
>> +         brw->scalar_vs = false;
>> +         prog_data->base.base.total_scratch = 0; /* reset */         
>> +      }
>>     }
>>  
>>     if (!assembly) {
>>