[Mesa-dev] [PATCH 7/7] i965: Don't spill "smeared" registers.

[Eric Anholt]

Paul Berry <stereotype441 at gmail.com> writes:

> On 19 September 2012 13:28, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
>> index e7f11ae..a819ae0 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp
>> @@ -317,11 +317,19 @@ fs_visitor::choose_spill_reg(struct ra_graph *g)
>>        for (unsigned int i = 0; i < 3; i++) {
>>          if (inst->src[i].file == GRF) {
>>             spill_costs[inst->src[i].reg] += loop_scale;
>> +
>> +            if (inst->src[i].smear >= 0) {
>> +               no_spill[inst->src[i].reg] = true;
>> +            }
>>
>
> Can we add a comment above the if statement to alert people to why we can't
> spill these registers (and why it's ok that we can't)?  Perhaps something
> like: "Register spilling logic assumes full-width registers; smeared
> registers have a width of 1 so if we try to spill them we'll generate
> invalid assembly.  This shouldn't be a problem because smeared registers
> are only used as short-term temporaries when loading pull constants, so
> spilling them is unlikely to reduce register pressure anyhow."
>
> I guess I shouldn't give this patch my "Reviewed-by" since I originated it,
> but assuming a comment is added, feel free to add:

Yeah, that makes me scared that our spilling choices are bogus.  I've
been worried about that -- on LM when I reduced the number of registers
used in its spilling shader, I got more spills happening, on some things
that looked like poor choices.  I did re-review the code, and didn't
find anything at the time.

-------------- 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/20120927/4285585c/attachment.pgp>