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

[Iago Toral]

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.
 */

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