[Mesa-dev] [PATCH 1/9] i965/vec4: Don't emit MOVs for unused URB slots.

[Iago Toral]

On Mon, 2015-10-19 at 21:08 -0700, Matt Turner wrote:
> Otherwise we'd emit a MOV from the null register (which isn't allowed).
> 
> Helps 24 programs in shader-db (the geometry shaders in GSCloth):
> 
> instructions in affected programs:     302 -> 262 (-13.25%)

Makes sense to me,

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

Out of curiosity, does moving from the null register have any actual
consequences?

Iago

> ---
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp    | 18 +++++++++++++-----
>  src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp |  2 +-
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> index f891910..c39f97e 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> @@ -1221,6 +1221,9 @@ vec4_visitor::emit_untyped_surface_read(unsigned surf_index, dst_reg dst,
>  void
>  vec4_visitor::emit_ndc_computation()
>  {
> +   if (output_reg[VARYING_SLOT_POS].file == BAD_FILE)
> +      return;
> +
>     /* Get the position */
>     src_reg pos = src_reg(output_reg[VARYING_SLOT_POS]);
>  
> @@ -1286,7 +1289,8 @@ vec4_visitor::emit_psiz_and_flags(dst_reg reg)
>         * Later, clipping will detect ucp[6] and ensure the primitive is
>         * clipped against all fixed planes.
>         */
> -      if (devinfo->has_negative_rhw_bug) {
> +      if (devinfo->has_negative_rhw_bug &&
> +          output_reg[BRW_VARYING_SLOT_NDC].file != BAD_FILE) {
>           src_reg ndc_w = src_reg(output_reg[BRW_VARYING_SLOT_NDC]);
>           ndc_w.swizzle = BRW_SWIZZLE_WWWW;
>           emit(CMP(dst_null_f(), ndc_w, src_reg(0.0f), BRW_CONDITIONAL_L));
> @@ -1334,8 +1338,10 @@ vec4_visitor::emit_generic_urb_slot(dst_reg reg, int varying)
>     assert(varying < VARYING_SLOT_MAX);
>     assert(output_reg[varying].type == reg.type);
>     current_annotation = output_reg_annotation[varying];
> -   /* Copy the register, saturating if necessary */
> -   return emit(MOV(reg, src_reg(output_reg[varying])));
> +   if (output_reg[varying].file != BAD_FILE)
> +      return emit(MOV(reg, src_reg(output_reg[varying])));
> +   else
> +      return NULL;
>  }
>  
>  void
> @@ -1354,11 +1360,13 @@ vec4_visitor::emit_urb_slot(dst_reg reg, int varying)
>     }
>     case BRW_VARYING_SLOT_NDC:
>        current_annotation = "NDC";
> -      emit(MOV(reg, src_reg(output_reg[BRW_VARYING_SLOT_NDC])));
> +      if (output_reg[BRW_VARYING_SLOT_NDC].file != BAD_FILE)
> +         emit(MOV(reg, src_reg(output_reg[BRW_VARYING_SLOT_NDC])));
>        break;
>     case VARYING_SLOT_POS:
>        current_annotation = "gl_Position";
> -      emit(MOV(reg, src_reg(output_reg[VARYING_SLOT_POS])));
> +      if (output_reg[VARYING_SLOT_POS].file != BAD_FILE)
> +         emit(MOV(reg, src_reg(output_reg[VARYING_SLOT_POS])));
>        break;
>     case VARYING_SLOT_EDGE:
>        /* This is present when doing unfilled polygons.  We're supposed to copy
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
> index 485a80e..5dd4f98 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
> @@ -217,7 +217,7 @@ vec4_vs_visitor::emit_urb_slot(dst_reg reg, int varying)
>         * shader.
>         */
>        vec4_instruction *inst = emit_generic_urb_slot(reg, varying);
> -      if (key->clamp_vertex_color)
> +      if (inst && key->clamp_vertex_color)
>           inst->saturate = true;
>        break;
>     }