On Mon, Apr 25, 2011 at 09:18:40AM -0700, Eric Anholt wrote: > On Sun, 24 Apr 2011 22:58:59 -0700, Tom Stellard <[email protected]> wrote: > > On Sun, Apr 24, 2011 at 02:00:40PM -0700, Eric Anholt wrote: > > > On Tue, 19 Apr 2011 23:09:46 -0700, Tom Stellard <[email protected]> > > > wrote: > > > > This function makes it possible to include input / payload registers in > > > > the interference graph. > > > > > > [...] > > > > > > > +/** > > > > + * This function allows a user to manually assign a register to a > > > > node. This > > > > + * is useful for nodes that belong to register classes that contain a > > > > very small > > > > + * number of registers that are not likely to be allocated if they end > > > > up at > > > > + * the bottom of the stack. > > > > + */ > > > > +void > > > > +ra_set_node_reg(struct ra_graph *g, unsigned int n, unsigned int reg) > > > > +{ > > > > + g->nodes[n].reg = reg; > > > > + g->nodes[n].in_stack = GL_FALSE; > > > > +} > > > > + > > > > > > So, this problem could also be solved by creating a register class per > > > payload register and including the nodes for the payload data in those > > > classes, right? However, I think you've got a good, safe solution for > > > some common uses that doesn't cause the size of the allocator data to > > > explode even more. > > > > > > > In order to include the payload registers in the interference graph, > > it is necessary to create classes for them *and* use ra_set_node_reg(). > > Simply creating classes for the payload registers won't work, because > > if the payload nodes aren't the first nodes popped off the stack, then > > the payload registers will be assigned to non-payload nodes before they > > can be assigned to the payload nodes. However, the payload nodes still > > need to be assigned to a class, otherwise the allocator will crash during > > the pq test. > > Yeah, for our driver we have a class of the actual registers, then other > classes for the weird things (pairs, aligned pairs, contiguous > bigger-than-pairs) that conflict with those base registers, so I was > reviewing on the assumption that those nodes would be of the base > register class but with their particular base register set.
We have 19 different classes plus the classes for the input registers. There is one class each for one, two, one + alpha, and two + alpha channel writes so we can do swizzle packing. Plus there is a class for every possible combination of channel writes (e.g. XY, XZ, X, etc.) for writemasks that we can't change due to constraints on some of the instructions. > > The hypothetical single-payload-register class nodes would never be > trivially colorable until they have no edges to any classes that could > interfere with their register, so they should be pushed to the stack > after all nodes that could interfere with it. Unless you're seeing > optimistic coloring just arbitrarily pushing? I'm interested in what > you saw when trying that, as it might point to a serious bug. I'm pretty sure the payload registers were only causing a problem with the optimistic coloring. -Tom > > Given that your patch makes optimistic coloring more likely, even more > reason to go with it, though. > _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
