[Mesa-dev] [PATCH 8/7] i965: Accurately bail on SIMD16 compiles.

[Kenneth Graunke]

On 03/12/2014 01:28 AM, Kenneth Graunke wrote:
> Ideally, we'd like to never even attempt the SIMD16 compile if we could
> know ahead of time that it won't succeed---it's purely a waste of time.
> This is especially important for state-based recompiles, which happen at
> draw time.
> 
> The fragment shader compiler has a number of checks like:
> 
>    if (dispatch_width == 16)
>       fail("...some reason...");
> 
> This patch introduces a new no16() function which replaces the above
> pattern.  In the SIMD8 compile, it sets a "SIMD16 will never work" flag.
> Then, brw_wm_fs_emit can check that flag, skip the SIMD16 compile, and
> issue a helpful performance warning if INTEL_DEBUG=perf is set.  (In
> SIMD16 mode, no16() calls fail(), for safety's sake.)
> 
> The great part is that this is not a heuristic---if the flag is set, we
> know with 100% certainty that the SIMD16 compile would fail.  (It might
> fail anyway if we run out of registers, but it's always worth trying.)
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp         | 69 +++++++++++++++++++++++-----
>  src/mesa/drivers/dri/i965/brw_fs.h           |  4 ++
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 44 +++++++++---------
>  3 files changed, 83 insertions(+), 34 deletions(-)
> 
> I forgot to send this one out...it applies on top of the previous 7 patches.
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 62848be..9ad80c5 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -647,9 +647,8 @@ fs_visitor::emit_shader_time_write(enum shader_time_shader_type type,
>  }
>  
>  void
> -fs_visitor::fail(const char *format, ...)
> +fs_visitor::vfail(const char *format, va_list va)
>  {
> -   va_list va;
>     char *msg;
>  
>     if (failed)
> @@ -657,9 +656,7 @@ fs_visitor::fail(const char *format, ...)
>  
>     failed = true;
>  
> -   va_start(va, format);
>     msg = ralloc_vasprintf(mem_ctx, format, va);
> -   va_end(va);
>     msg = ralloc_asprintf(mem_ctx, "FS compile failed: %s\n", msg);
>  
>     this->fail_msg = msg;
> @@ -669,6 +666,49 @@ fs_visitor::fail(const char *format, ...)
>     }
>  }
>  
> +void
> +fs_visitor::fail(const char *format, ...)
> +{
> +   va_list va;
> +
> +   va_start(va, format);
> +   vfail(format, va);
> +   va_end(va);
> +}
> +
> +/**
> + * Mark this program as impossible to compile in SIMD16 mode.
> + *
> + * During the SIMD8 compile (which happens first), we can detect and flag
> + * things that are unsupported in SIMD16 mode, so the compiler can skip
> + * the SIMD16 compile altogether.
> + *
> + * During a SIMD16 compile (if one happens anyway), this just calls fail().
> + */
> +void
> +fs_visitor::no16(const char *format, ...)
> +{
> +   va_list va;
> +
> +   va_start(va, format);
> +
> +   if (dispatch_width == 16) {
> +      vfail(format, va);
> +      return;
> +   }
> +
> +   simd16_unsupported = true;
> +
> +   if (INTEL_DEBUG & DEBUG_PERF) {
> +      if (no16_msg)
> +         ralloc_vasprintf_append(&no16_msg, format, va);
> +      else
> +         no16_msg = ralloc_vasprintf(mem_ctx, format, va);
> +   }
> +
> +   va_end(va);
> +}
> +
>  fs_inst *
>  fs_visitor::emit(enum opcode opcode)
>  {
> @@ -1356,8 +1396,8 @@ fs_visitor::emit_math(enum opcode opcode, fs_reg dst, fs_reg src0, fs_reg src1)
>     switch (opcode) {
>     case SHADER_OPCODE_INT_QUOTIENT:
>     case SHADER_OPCODE_INT_REMAINDER:
> -      if (brw->gen >= 7 && dispatch_width == 16)
> -	 fail("SIMD16 INTDIV unsupported\n");
> +      if (brw->gen >= 7)
> +	 no16("SIMD16 INTDIV unsupported\n");
>        break;
>     case SHADER_OPCODE_POW:
>        break;
> @@ -3504,13 +3544,18 @@ brw_wm_fs_emit(struct brw_context *brw, struct brw_wm_compile *c,
>     exec_list *simd16_instructions = NULL;
>     fs_visitor v2(brw, c, prog, fp, 16);
>     if (brw->gen >= 5 && likely(!(INTEL_DEBUG & DEBUG_NO16))) {
> -      /* Try a SIMD16 compile */
> -      v2.import_uniforms(&v);
> -      if (!v2.run()) {
> -         perf_debug("SIMD16 shader failed to compile, falling back to "
> -                    "SIMD8 at a 10-20%% performance cost: %s", v2.fail_msg);
> +      if (!v.simd16_unsupported) {
> +         /* Try a SIMD16 compile */
> +         v2.import_uniforms(&v);
> +         if (!v2.run()) {
> +            perf_debug("SIMD16 shader failed to compile, falling back to "
> +                       "SIMD8 at a 10-20%% performance cost: %s", v2.fail_msg);
> +         } else {
> +            simd16_instructions = &v2.instructions;
> +         }
>        } else {
> -         simd16_instructions = &v2.instructions;
> +         perf_debug("SIMD16 shader unsupported, falling back to "
> +                    "SIMD8 at a 10-20%% performance cost: %s", v.no16_msg);
>        }
>     }
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 9de1f3a..0d064f6 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -381,7 +381,9 @@ public:
>     void insert_gen4_send_dependency_workarounds();
>     void insert_gen4_pre_send_dependency_workarounds(fs_inst *inst);
>     void insert_gen4_post_send_dependency_workarounds(fs_inst *inst);
> +   void vfail(const char *msg, va_list args);
>     void fail(const char *msg, ...);
> +   void no16(const char *msg, ...);
>     void lower_uniform_pull_constant_loads();
>  
>     void push_force_uncompressed();
> @@ -541,6 +543,8 @@ public:
>  
>     bool failed;
>     char *fail_msg;
> +   bool simd16_unsupported;
> +   char *no16_msg;
>  
>     /* Result of last visit() method. */
>     fs_reg result;

I neglected to to initialize these, so SIMD16 compiles would randomly
fail for no reason.  I've fixed that in the newest version.

--Ken


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140315/ca32903c/attachment-0001.sig>