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

[Iago Toral]

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?

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);