[Mesa-dev] [PATCH] i965: move vs outputs written into a helper

Iago Toral itoral at igalia.com
Wed Jun 22 07:31:28 UTC 2016


On Wed, 2016-06-22 at 11:34 +1000, Timothy Arceri wrote:
> On Mon, 2016-06-20 at 17:38 +0200, Iago Toral wrote:
> > On Mon, 2016-06-20 at 21:11 +1000, Timothy Arceri wrote:
> > > 
> > > We will reuse this for fs key generation for the on disk shader
> > > cache.
> > > ---
> > >  src/mesa/drivers/dri/i965/brw_vs.c | 72 ++++++++++++++++++++++--
> > > --------------
> > >  src/mesa/drivers/dri/i965/brw_vs.h |  4 +++
> > >  2 files changed, 45 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
> > > b/src/mesa/drivers/dri/i965/brw_vs.c
> > > index e40885c..8b99db6 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_vs.c
> > > +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> > > @@ -43,6 +43,45 @@
> > >  
> > >  #include "util/ralloc.h"
> > >  
> > > +GLbitfield64
> > > +brw_vs_outputs_written(struct brw_context *brw, struct
> > > brw_vs_prog_key *key,
> > > +                       GLbitfield64 outputs_written)
> > Not sure about the name since you already pass the outputs written
> > and
> > this helper only amends the bitfield with some special outputs...
> > maybe
> > something like brw_vs_amend_outputs_written would be more accurate.
> > What
> > do you think?
> 
> How about something like this?
> 
> brw_vs_outputs_written(struct brw_context *brw, struct, brw_vs_prog_key
> *key, GLbitfield64 user_varyings)
> {
>    GLbitfield64 outputs_written = user_varyings;

Sure, that works for me too. Thanks!

> > 
> > Either way:
> > Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> > 
> > > 
> > > +{
> > > +   if (key->copy_edgeflag) {
> > > +      outputs_written |= BITFIELD64_BIT(VARYING_SLOT_EDGE);
> > > +   }
> > > +
> > > +   if (brw->gen < 6) {
> > > +      /* Put dummy slots into the VUE for the SF to put the
> > > replaced
> > > +       * point sprite coords in.  We shouldn't need these dummy
> > > slots,
> > > +       * which take up precious URB space, but it would mean that
> > > the SF
> > > +       * doesn't get nice aligned pairs of input coords into
> > > output
> > > +       * coords, which would be a pain to handle.
> > > +       */
> > > +      for (unsigned i = 0; i < 8; i++) {
> > > +         if (key->point_coord_replace & (1 << i))
> > > +            outputs_written |= BITFIELD64_BIT(VARYING_SLOT_TEX0 +
> > > i);
> > > +      }
> > > +
> > > +      /* if back colors are written, allocate slots for front
> > > colors too */
> > > +      if (outputs_written & BITFIELD64_BIT(VARYING_SLOT_BFC0))
> > > +         outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL0);
> > > +      if (outputs_written & BITFIELD64_BIT(VARYING_SLOT_BFC1))
> > > +         outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL1);
> > > +   }
> > > +
> > > +   /* In order for legacy clipping to work, we need to populate
> > > the clip
> > > +    * distance varying slots whenever clipping is enabled, even if
> > > the vertex
> > > +    * shader doesn't write to gl_ClipDistance.
> > > +    */
> > > +   if (key->nr_userclip_plane_consts > 0) {
> > > +      outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST0);
> > > +      outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST1);
> > > +   }
> > > +
> > > +   return outputs_written;
> > > +}
> > > +
> > >  bool
> > >  brw_codegen_vs_prog(struct brw_context *brw,
> > >                      struct gl_shader_program *prog,
> > > @@ -55,7 +94,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
> > >     struct brw_vs_prog_data prog_data;
> > >     struct brw_stage_prog_data *stage_prog_data =
> > > &prog_data.base.base;
> > >     void *mem_ctx;
> > > -   int i;
> > >     struct brw_shader *vs = NULL;
> > >     bool start_busy = false;
> > >     double start_time = 0;
> > > @@ -108,11 +146,11 @@ brw_codegen_vs_prog(struct brw_context *brw,
> > >                                   &prog_data.base.base);
> > >     }
> > >  
> > > -   GLbitfield64 outputs_written = vp->program.Base.OutputsWritten;
> > > +   GLbitfield64 outputs_written =
> > > +      brw_vs_outputs_written(brw, key, vp-
> > > >program.Base.OutputsWritten);
> > >     prog_data.inputs_read = vp->program.Base.InputsRead;
> > >  
> > >     if (key->copy_edgeflag) {
> > > -      outputs_written |= BITFIELD64_BIT(VARYING_SLOT_EDGE);
> > >        prog_data.inputs_read |= VERT_BIT_EDGEFLAG;
> > >     }
> > >  
> > > @@ -120,34 +158,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
> > >        ((1 << vp->program.Base.CullDistanceArraySize) - 1) <<
> > >        vp->program.Base.ClipDistanceArraySize;
> > >  
> > > -   if (brw->gen < 6) {
> > > -      /* Put dummy slots into the VUE for the SF to put the
> > > replaced
> > > -       * point sprite coords in.  We shouldn't need these dummy
> > > slots,
> > > -       * which take up precious URB space, but it would mean that
> > > the SF
> > > -       * doesn't get nice aligned pairs of input coords into
> > > output
> > > -       * coords, which would be a pain to handle.
> > > -       */
> > > -      for (i = 0; i < 8; i++) {
> > > -         if (key->point_coord_replace & (1 << i))
> > > -            outputs_written |= BITFIELD64_BIT(VARYING_SLOT_TEX0 +
> > > i);
> > > -      }
> > > -
> > > -      /* if back colors are written, allocate slots for front
> > > colors too */
> > > -      if (outputs_written & BITFIELD64_BIT(VARYING_SLOT_BFC0))
> > > -         outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL0);
> > > -      if (outputs_written & BITFIELD64_BIT(VARYING_SLOT_BFC1))
> > > -         outputs_written |= BITFIELD64_BIT(VARYING_SLOT_COL1);
> > > -   }
> > > -
> > > -   /* In order for legacy clipping to work, we need to populate
> > > the clip
> > > -    * distance varying slots whenever clipping is enabled, even if
> > > the vertex
> > > -    * shader doesn't write to gl_ClipDistance.
> > > -    */
> > > -   if (key->nr_userclip_plane_consts > 0) {
> > > -      outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST0);
> > > -      outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST1);
> > > -   }
> > > -
> > >     brw_compute_vue_map(brw->intelScreen->devinfo,
> > >                         &prog_data.base.vue_map, outputs_written,
> > >                         prog ? prog->SeparateShader ||
> > > diff --git a/src/mesa/drivers/dri/i965/brw_vs.h
> > > b/src/mesa/drivers/dri/i965/brw_vs.h
> > > index 15ac048..25241a9 100644
> > > --- a/src/mesa/drivers/dri/i965/brw_vs.h
> > > +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> > > @@ -51,6 +51,10 @@
> > >  extern "C" {
> > >  #endif
> > >  
> > > +GLbitfield64
> > > +brw_vs_outputs_written(struct brw_context *brw, struct
> > > brw_vs_prog_key *key,
> > > +                       GLbitfield64 outputs_written);
> > > +
> > >  void brw_vs_debug_recompile(struct brw_context *brw,
> > >                              struct gl_shader_program *prog,
> > >                              const struct brw_vs_prog_key *key);
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 




More information about the mesa-dev mailing list