[Mesa-dev] [PATCH 11/15] i965: abort linking if we exhaust the registers

[Juan A. Suarez Romero]

On Fri, 2016-04-29 at 11:15 +0200, Ian Romanick wrote:
> The driver supports up to 16 vertex attributes.
> > ARB_vertex_attrib_64bit
> > states that attribute variables of type dvec3, dvec4, dmat2x3,
> > dmat2x4,
> > dmat3, dmat3x4, dmat4x3, and dmat4 *may* count as consuming twice
> > as
> > many attributes as equivalent single-precision types.
> > 
> > 
> > I highlight the may, because it is not mandatory. If we count those
> > types as consuming the same as a single-precision type (which is
> > what
> > is happening in Mesa), we are consuming 15 attributes, so we are
> > under 
> > the limit.
> This is the thing we need to fix.  Bailing from deep inside the
> driver
> code generation (which may happen long, long after linking) is not
> allowed.  If a shader is not going to work, we are required to
> generate
> the error in glLinkProgram.
> 

I'm not sure if I am following you. In which cases there can be code
generation after the linking?

On the other side, the error is generated when calling glLinkProgram():
it happens inside the stack that follows the call, when the NIR code is
transformed in the intermediate BRW code.

So from user pov, the error happens when calling glLinkProgram(), not
afterwards.


> > > But I see couple of drawbacks with this approach:
> > > 
> > > 
> > > - There are tests that under the same conditions (less than the
> > limit
> > > if you count those types as occupying the same as single-
> > precision, but
> > > beyond the limit if those types are considered as consuming
> > twice) they
> > > still works. An example is the attached shader2 test: it requires
> > 13
> > > attributes (or 19 counting as twice the mentioned types) and it
> > works
> > > fine.
> > >
> > - This check affects to all the backends. And there could be some
> > backend that works perfectly fine with the current implementation,
> > which is less conservative. In fact, we have an example: the same
> > driver running in vec4 mode (SIMD4x2) works perfectly fine.
> I think we can handle this by having a per-type (double, dvec2,
> dvec3,
> and dvec4) flag to select the double or don't-double behavior.
> 

Do you mean only dvec3 and dvec4 (and types based on those: dmat2x3,
dmat2x4, dmat3, dmat3x4, dmat4x3, and dmat4)?

Because only restrict to those types to count them as twice when
checking if we bypass the max attributes limit.

Also, do we really need a flag per type? Wouldn't it be enough to have
a single flag stating if all of those types are counted twice or not?

Finally, I understand the idea is that the flag would be like a boolean
with true (count twice) or false (don't count twice), contained in each
backend/driver (like the max attribs limit, for instance).


Anyway, this would fix the second problem, but not the former: if our
gen8 backend counts doubles as twice, it will prevent to run some
shaders that otherway would work fine (like the shader2). If this is an
acceptable solution (shaders that work fine now will be rejected
because they use too much vertex attribs), then it's fine.


	J.A.