[Mesa-dev] [PATCH 2/2] i965/vs: Implement register spilling.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> To validate this code, I ran piglit -t vs quick.tests with the "go spill
> everything" debugging code enabled.  There was only one regression:
> glsl-vs-unroll-explosion simply ran out of registers.  This should be
> fine in the real world, since no one actually spills every single
> register.
>
> NOTE: This is a candidate for the 9.0 branch. Even if it proves to have
> bugs, it's likely better than simply failing to compile.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_vec4.h               |   3 +
>  src/mesa/drivers/dri/i965/brw_vec4_emit.cpp        |  14 +++
>  .../drivers/dri/i965/brw_vec4_reg_allocate.cpp     | 128 ++++++++++++++++++++-
>  3 files changed, 144 insertions(+), 1 deletion(-)
>
> Unfortunately, this assertion fails on the one game that needs it;
> oglconform's vertexshader advanced.TestLights test also hits the same
> assertion failure (and I would have expected this to fix that test).

I've got a followon patch to fix the assertion failure that I think
could be put in ahead of this one.  It doesn't fix the failure to
register allocate, because the live interval analysis is too dumb and
spilling things inside a loop just results in more pressure since all
things used in a loop are live across the entire loop.

> +void
> +vec4_visitor::spill_reg(int spill_reg_nr)
> +{
> +   assert(virtual_grf_sizes[spill_reg_nr] == 1);
> +   unsigned int spill_offset = c->last_scratch++;
> +
> +   /* Generate spill/unspill instructions for the objects being spilled. */
> +   foreach_list(node, &this->instructions) {
> +      vec4_instruction *inst = (vec4_instruction *) node;
> +
> +      for (unsigned int i = 0; i < 3; i++) {
> +         if (inst->src[i].file == GRF && inst->src[i].reg == spill_reg_nr) {
> +            src_reg spill_reg = inst->src[i];
> +            inst->src[i].reg = virtual_grf_alloc(1);
> +            dst_reg temp = dst_reg(inst->src[i]);
> +
> +            /* Only read the necessary channels, to avoid overwriting the rest
> +             * with data that may not have been written to scratch.
> +             */
> +            temp.writemask = 0;
> +            for (int c = 0; c < 4; c++)
> +               temp.writemask |= (1 << BRW_GET_SWZ(inst->src[i].swizzle, c));
> +            assert(temp.writemask != 0);
> +
> +            emit_scratch_read(inst, temp, spill_reg, spill_offset);
> +         }

temp is otherwise full of garbage and not used by any other instruction,
so I don't think you care about carefully writemasking the read into it.

Other than that, the series is Reviewed-by: Eric Anholt <eric at anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20121003/9d8b0698/attachment.pgp>