Re: [Mesa-dev] [PATCH 08/15] i965/vec4: use attribute slots to calculate URB read length

[Kenneth Graunke]

On Tuesday, May 10, 2016 6:24:39 PM PDT Juan A. Suarez Romero wrote:
> On Mon, 2016-05-09 at 23:37 -0700, Kenneth Graunke wrote:
> > void *log_data,
> > > const unsigned *assembly = NULL;
> > >  
> > > unsigned nr_attributes = _mesa_bitcount_64(prog_data-
> > >inputs_read);
> > > +   unsigned nr_attribute_slots = 0;
> > 
> > Can't you just do:
> > 
> > nir_shader *nir = vp->program.Base.nir;
> > unsigned nr_attribute_slots =
> >_mesa_bitcount_64(nir->info.inputs_read) +
> >_mesa_bitcount_64(nir->info.double_inputs_read);
> > 
> > It seems like that should work without the need to iterate variables.
> 
> 
> Right. I'll use that instead. Thank you!
> 
>   J.A.
> 

Cool!  Have a:
Reviewed-by: Kenneth Graunke 


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/15] i965/vec4: use attribute slots to calculate URB read length

[Juan A. Suarez Romero]

On Mon, 2016-05-09 at 23:37 -0700, Kenneth Graunke wrote:
> void *log_data,
> > const unsigned *assembly = NULL;
> >  
> > unsigned nr_attributes = _mesa_bitcount_64(prog_data-
> >inputs_read);
> > +   unsigned nr_attribute_slots = 0;
> 
> Can't you just do:
> 
>     nir_shader *nir = vp->program.Base.nir;
>     unsigned nr_attribute_slots =
>    _mesa_bitcount_64(nir->info.inputs_read) +
>    _mesa_bitcount_64(nir->info.double_inputs_read);
> 
> It seems like that should work without the need to iterate variables.


Right. I'll use that instead. Thank you!

J.A.


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 08/15] i965/vec4: use attribute slots to calculate URB read length

[Kenneth Graunke]

On Thursday, April 28, 2016 1:40:38 PM PDT Antia Puentes wrote:
> From: "Juan A. Suarez Romero" 
> 
> Do not use total attributes because a dvec3/dvec4 attribute requires two
> slots. So rather use total attribute slots.
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.cpp | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/
i965/brw_vec4.cpp
> index 4766be0..005760d 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -2107,6 +2107,7 @@ brw_compile_vs(const struct brw_compiler *compiler, 
void *log_data,
> const unsigned *assembly = NULL;
>  
> unsigned nr_attributes = _mesa_bitcount_64(prog_data->inputs_read);
> +   unsigned nr_attribute_slots = 0;

Can't you just do:

nir_shader *nir = vp->program.Base.nir;
unsigned nr_attribute_slots =
   _mesa_bitcount_64(nir->info.inputs_read) +
   _mesa_bitcount_64(nir->info.double_inputs_read);

It seems like that should work without the need to iterate variables.

>  
> /* gl_VertexID and gl_InstanceID are system values, but arrive via an
>  * incoming vertex attribute.  So, add an extra slot.
> @@ -2117,11 +2118,23 @@ brw_compile_vs(const struct brw_compiler *compiler, 
void *log_data,
>  BITFIELD64_BIT(SYSTEM_VALUE_VERTEX_ID_ZERO_BASE) |
>  BITFIELD64_BIT(SYSTEM_VALUE_INSTANCE_ID))) {
>nr_attributes++;
> +  nr_attribute_slots++;
> }
>  
> /* gl_DrawID has its very own vec4 */
> if (shader->info.system_values_read & 
BITFIELD64_BIT(SYSTEM_VALUE_DRAW_ID)) {
>nr_attributes++;
> +  nr_attribute_slots++;
> +   }
> +
> +   GLbitfield64 processed_attributes = 0;
> +   foreach_list_typed(nir_variable, var, node, >inputs) {
> +  /* Only interested in values not already processed */
> +  if (processed_attributes & BITFIELD64_BIT(var->data.location))
> + continue;
> +
> +  nr_attribute_slots += glsl_count_attribute_slots(var->type, false);

If we do need to keep this code, could we use type_size_vec4 here?
It looks weird (although correct) to call glsl_count_attribute_slots
with the is_vs_input parameter set to false...for VS inputs... :)

> +  processed_attributes |= BITFIELD64_BIT(var->data.location);
> }
>  
> /* The 3DSTATE_VS documentation lists the lower bound on "Vertex URB 
Entry
> @@ -2129,9 +2142,11 @@ brw_compile_vs(const struct brw_compiler *compiler, 
void *log_data,
>  * vec4 mode, the hardware appears to wedge unless we read something.
>  */
> if (is_scalar)
> -  prog_data->base.urb_read_length = DIV_ROUND_UP(nr_attributes, 2);
> +  prog_data->base.urb_read_length =
> + DIV_ROUND_UP(nr_attribute_slots, 2);
> else
> -  prog_data->base.urb_read_length = DIV_ROUND_UP(MAX2(nr_attributes, 
1), 2);
> +  prog_data->base.urb_read_length =
> + DIV_ROUND_UP(MAX2(nr_attribute_slots, 1), 2);
>  
> prog_data->nr_attributes = nr_attributes;
>  
> @@ -2140,7 +2155,7 @@ brw_compile_vs(const struct brw_compiler *compiler, 
void *log_data,
>  * the larger of the two.
>  */
> const unsigned vue_entries =
> -  MAX2(nr_attributes, (unsigned)prog_data->base.vue_map.num_slots);
> +  MAX2(nr_attribute_slots, (unsigned)prog_data-
>base.vue_map.num_slots);
>  
> if (compiler->devinfo->gen == 6)
>prog_data->base.urb_entry_size = DIV_ROUND_UP(vue_entries, 8);
> 



signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev