[Mesa-dev] [PATCH 07/37] i965/gen6/gs: Implement GS_OPCODE_FF_SYNC.

[Iago Toral Quiroga]

On lun, 2014-09-01 at 11:17 -0700, Kenneth Graunke wrote:
> On Thursday, August 14, 2014 01:11:39 PM Iago Toral Quiroga wrote:
> > This implements the FF_SYNC message required in gen6  geometry shaders to
> > get the initial URB handle.
> > ---
> >  src/mesa/drivers/dri/i965/brw_defines.h          | 14 +++++++++
> >  src/mesa/drivers/dri/i965/brw_shader.cpp         |  2 ++
> >  src/mesa/drivers/dri/i965/brw_vec4.h             |  3 ++
> >  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 40 ++++++++++++++++++++++++
> >  4 files changed, 59 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> > index 3564041..125d728 100644
> > --- a/src/mesa/drivers/dri/i965/brw_defines.h
> > +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> > @@ -1002,6 +1002,20 @@ enum opcode {
> >      * - dst is the GRF for gl_InvocationID.
> >      */
> >     GS_OPCODE_GET_INSTANCE_ID,
> > +
> > +   /**
> > +    * Send a FF_SYNC message to allocate initial URB handles (gen6).
> > +    *
> > +    * - dst will hold the newly allocated VUE handle. It is expected to be
> > +    *   be initialized so that it can be used to as the FF_SYNC message header
> > +    *   (that is, it won't do an implied move from R0).
> > +    *
> > +    * - src0 is a temporary that will be used as writeback register for the
> > +    *   FF_SYNC operation.
> 
> I was originally a bit concerned that writing over a source register could get your into trouble - say, optimization passes not noticing that you're modifying src0, or the scheduler not noticing dependencies.  But, since you allocate a temporary, and only use it in this one instruction, there's not really any chance for optimizations to combine it with anything.
> 
> So, I suppose this will work.
> 
> Another way you might consider doing this...
> 
> 1. Add GS_OPCODE_FF_SYNC to vec4_visitor::implied_mrf_writes, and return 1.
>    This tells the scheduler etc. that you implicitly trash 1 MRF.
> 2. Make the visitor set inst->base_mrf, and MOV <base_mrf> g0.
> 3. Make the generator use brw_message_reg(inst->base_mrf) for the header MOVs.
> 
> Then, src0 can be the number of primitives written, and you don't need src1.
> That seems simpler to me.
> 
> brw_ff_sync will resolve your GRF (dst) to an MRF anyway; the above approach would write directly into the MRF, saving you a GRF.
> 
> Thoughts?

That looks like a better idea. I'll give it a go.
Thanks!

Iago