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

[Kenneth Graunke]

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.

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) {
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150603/bfdbc351/attachment.sig>