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

[Kenneth Graunke]

On Friday, July 03, 2015 11:11:45 AM Pohjolainen, Topi wrote:
> 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.

The interface is indeed weird.  reg_allocate() may fail to allocate
without spilling, at which point it will spill a register, and return
false.  The caller is expected to call it again to retry.

I don't know why it doesn't just do that itself.

> 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);
> >  
> 
-------------- 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/20150703/15655b62/attachment.sig>