[Mesa-dev] [PATCH 2/6] i965/vec4: Move perf_debug about register spilling into the visitor.

[Pohjolainen, Topi]

On Wed, Jul 01, 2015 at 03:03:32PM -0700, Kenneth Graunke wrote:
> This patch makes us only issue the performance warning about register
> spilling if we actually spilled registers.  We also use scratch space
> for indirect addressing and the like.
> 
> This is basically commit c51163b0cf7aff0375b1a5ea4cb3da9d9e164044 for
> the vec4 backend.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_gs.c     |  4 ----
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 16 +++++++++++++---
>  src/mesa/drivers/dri/i965/brw_vs.c     |  4 ----
>  3 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> index 52c7303..7f947e0 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -268,10 +268,6 @@ brw_codegen_gs_prog(struct brw_context *brw,
>  
>     /* Scratch space is used for register spilling */
>     if (c.base.last_scratch) {
> -      perf_debug("Geometry shader triggered register spilling.  "
> -                 "Try reducing the number of live vec4 values to "
> -                 "improve performance.\n");
> -
>        c.prog_data.base.base.total_scratch
>           = brw_get_scratch_size(c.base.last_scratch*REG_SIZE);
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 2a56564..60f73e2 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1827,9 +1827,19 @@ vec4_visitor::run(gl_clip_plane *clip_planes)
>        }
>     }
>  
> -   while (!reg_allocate()) {
> -      if (failed)
> -         return false;
> +   bool allocated_without_spills = reg_allocate();
> +
> +   if (!allocated_without_spills) {
> +      compiler->shader_perf_log(log_data,
> +                                "%s shader triggered register spilling.  "
> +                                "Try reducing the number of live vec4 values "
> +                                "to improve performance.\n",
> +                                stage_name);
> +
> +      while (!reg_allocate()) {

I tried to understand a little how repeating calls to reg_allocate() differ
from previous in result wise. I didn't really get it but that doesn't really
prevent me from reviewing this patch. This patch preserves the logic while
corresponding to the intent in commit message.

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

> +         if (failed)
> +            return false;
> +      }
>     }
>  
>     opt_schedule_instructions();
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index 6e9848f..edbcbcf 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -196,10 +196,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
>  
>     /* Scratch space is used for register spilling */
>     if (c.base.last_scratch) {
> -      perf_debug("Vertex shader triggered register spilling.  "
> -                 "Try reducing the number of live vec4 values to "
> -                 "improve performance.\n");
> -
>        prog_data.base.base.total_scratch
>           = brw_get_scratch_size(c.base.last_scratch*REG_SIZE);
>  
> -- 
> 2.4.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev