Paul Berry <stereotype...@gmail.com> writes:
> On 19 September 2012 13:28, Kenneth Graunke <kenn...@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.
pgpjDdtYkALSD.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
