[Mesa-dev] [PATCH 12/15] i965: abort linking if URB read length greater than 15

[Juan A. Suarez Romero]

On Wed, 2016-05-04 at 03:43 -0700, Kenneth Graunke wrote:
> On Thursday, April 28, 2016 1:40:42 PM PDT Antia Puentes wrote:
> > 
> > From: "Juan A. Suarez Romero" <jasuarez at igalia.com>
> > 
> > In scalar mode, URB read length limit is 15. Abort if we go beyond
> > it.
> > ---
> >  src/mesa/drivers/dri/i965/brw_vec4.cpp | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > b/src/mesa/drivers/dri/
> i965/brw_vec4.cpp
> > 
> > index 9816f0d..04287fb 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> > @@ -2148,6 +2148,14 @@ brw_compile_vs(const struct brw_compiler
> > *compiler, 
> void *log_data,
> > 
> >        prog_data->base.urb_read_length =
> >           DIV_ROUND_UP(MAX2(nr_attribute_slots, 1), 2);
> >  
> > +   if (is_scalar && prog_data->base.urb_read_length > 15) {
> > +      if (error_str)
> > +         *error_str = ralloc_strdup(mem_ctx,
> > +                                    "Too many attributes. Try to
> > reduce the 
> "
> > 
> > +                                    "number of attributes or their
> > size");
> > +      return NULL;
> > +   }
> > +
> >     prog_data->nr_attributes = nr_attributes;
> >     prog_data->nr_attribute_slots = nr_attribute_slots;
> >  
> > 
> Have you hit this limit?  I'm probably doing my math wrong, but it
> doesn't seem like we should ever hit it...
> 


Yes. We have a piglit test (not in master yet) that hits the limit. You
can find it in http://pastebin.com/k96EyYun


This links with a comment made by Ian[1] in a different patch, but that
also applies here.

https://lists.freedesktop.org/archives/mesa-dev/2016-April/114885.html

dvec3/dmat*3/dvec4/dmat*4 can be counted as consuming the same
attributes as the single precision version, or consuming twice. Current
code follows the former option. If we switch to the second, then we
won't hit this limit. But this has other cons, as explained in the
thread.


> I'm not sure we can legally just say no - we may need to resort to
> pulling URB data...
> 

Maybe. Another option could be to switch to vec4... if gen8+vec4 were
working.

As this seems an extreme case (when we use lot of attributes), I
thought that dealing with it was more like a task to improve in the
future.


	J.A.