[Mesa-dev] [PATCH 28/57] i965/fs: Take into account copy register offset during compute-to-mrf.

[Francisco Jerez]

Iago Toral <itoral at igalia.com> writes:

> On Wed, 2016-09-07 at 18:48 -0700, Francisco Jerez wrote:
>> This was dropping 'inst->dst.offset' on the floor.  Nothing in the
>> code above seems to guarantee that it's zero and in that case the
>> offset of the register being coalesced into wouldn't be taken into
>> account while rewriting the generating instruction.
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs.cpp | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 98da06c..5b7ca98 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -2821,7 +2821,7 @@ fs_visitor::compute_to_mrf()
>>              }
>>  
>>              scan_inst->dst.file = MRF;
>> -            scan_inst->dst.offset %= REG_SIZE;
>> +            scan_inst->dst.offset = inst->dst.offset + scan_inst-
>> >dst.offset % REG_SIZE;
>
> So if we had something like this:
>
> 0: mov(4) r1.4:F r0.0:F
> 1: mov(4) m1:F r1.4:F
>
> This pass is trying to get us:
>
> mov(4) m1:F r0.0:F
>

Yeah, that's what it should be doing, but the current code (already
before this series) would give you:

| mov(4) m1.4:F r0.0:F

which is pretty bogus.

> In that case, the offset into the destination of the instruction we are propagating from should not affect the offset into the MRF we are writing to. Or maybe I am missing what is going on here :)

Yeah, that's exactly the problem fixed in the next patch.  Note that
this commit doesn't actually introduce the bug, it's preserving the
buggy behavior of preexisting code, which happens to become more obvious
due to the explicit 'scan_inst->dst.offset % REG_SIZE' term instead of
the '%=' operator.

>
>>              scan_inst->saturate |= inst->saturate;
>>              if (!regs_left)
>>                 break;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160909/bc824ae4/attachment.sig>