[Mesa-dev] [PATCH 2/3] i965: Make split_virtual_grfs() call compact_virtual_grfs().

[Iago Toral]

On Mon, 2016-10-24 at 11:34 +0200, Iago Toral wrote:
> On Sun, 2016-10-23 at 23:44 -0700, Kenneth Graunke wrote:
> > 
> > Post-splitting, VGRFs have a maximum size (MAX_VGRF_SIZE).  This is
> > required by the register allocator, as we have to create classes
> > for
> > each size of VGRF.
> > 
> > We can (and do) allocate virtual registers larger than
> > MAX_VGRF_SIZE,
> > but we must ensure that they are splittable.  split_virtual_grfs()
> > asserts that the post-splitting register size is in range.
> > 
> > Unfortunately, these trip for completely dead registers which are
> > too
> > large - we only set split points for live registers.  So dead ones
> > are
> > never split, and if they happened to be too large, they'd trip
> > asserts.
> > 
> > To fix this, call compact_virtual_grfs() to eliminate dead
> > registers
> > before splitting.
> > 
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index 1c7a6e6..31d21ec 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -1687,6 +1687,8 @@ fs_visitor::assign_gs_urb_setup()
> >  void
> >  fs_visitor::split_virtual_grfs()
> >  {
> Maybe I would add a comment here to explain why we compact before we
> split so people don't forget about the special case this handles.
> Maybe
> something like this:
> 
> /* Compact the register file so we eliminate dead vgrfs. This
>  * only defines split points for live registers, so if we have
>  * too large dead registers they will hit assertions later.
>  */

Also, with or without the comment:

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


> > 
> > +   compact_virtual_grfs();
> > +
> >     int num_vars = this->alloc.count;
> >  
> >     /* Count the total number of registers */