[Mesa-dev] [PATCH v2 08/23] i965/vs: Fix a subtlety in the nr_attributes == 0 workaround.

[Iago Toral]

On Mon, 2015-10-05 at 13:41 -0700, Kenneth Graunke wrote:
> nr_attributes is used to compute first_non_payload_grf, which is the
> first register we're allowed to use for ordinary register allocation.
> 
> The hardware requires us to read at least one pair of values, but we're
> completely free to overwrite that garbage register with whatever we like.
> 
> Instead of altering nr_attributes, we should alter urb_read_length, which
> only affects the amount we ask the VF to read.  This should save us a
> register in trivial cases (which admittedly isn't very useful).
> 
> While we're at it, improve the explanation in the comments.
> 
> v2: Actually do what I said (caught by Ilia).

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

> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_vs.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> Ilia pointed out on IRC that my previous version of this patch was totally
> bunk - I kept editing nr_attributes, and actually made the SIMD8 case start
> using a read length of 1.
> 
> Here's one that actually does what I meant.  I verified that it actually
> keeps using a URB Read Length of 0 in the SIMD8 VS.  I also verified that
> the previous patch doesn't break that either, so I think it's OK after all.
> 
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index f585f22..677a58f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -195,14 +195,16 @@ brw_codegen_vs_prog(struct brw_context *brw,
>        nr_attributes++;
>     }
>  
> -   /* The BSpec says we always have to read at least one thing from the VF,
> -    * and it appears that the hardware wedges otherwise.
> +   /* The 3DSTATE_VS documentation lists the lower bound on "Vertex URB Entry
> +    * Read Length" as 1 in vec4 mode, and 0 in SIMD8 mode.  Empirically, in
> +    * vec4 mode, the hardware appears to wedge unless we read something.
>      */
> -   if (nr_attributes == 0 && !brw->intelScreen->compiler->scalar_vs)
> -      nr_attributes = 1;
> +   if (brw->intelScreen->compiler->scalar_vs)
> +      prog_data.base.urb_read_length = DIV_ROUND_UP(nr_attributes, 2);
> +   else
> +      prog_data.base.urb_read_length = DIV_ROUND_UP(MAX2(nr_attributes, 1), 2);
>  
>     prog_data.nr_attributes = nr_attributes;
> -   prog_data.base.urb_read_length = DIV_ROUND_UP(nr_attributes, 2);
>  
>     /* Since vertex shaders reuse the same VUE entry for inputs and outputs
>      * (overwriting the original contents), we need to make sure the size is