On 03/05/16 20:30, Jordan Justen wrote:
> On 2016-05-03 05:21:51, Samuel Iglesias Gonsálvez wrote:
>> From: Iago Toral Quiroga <ito...@igalia.com>
>>
>> We should not offset into them based on the relative offset of
>> our source and the destination of the instruction we are copy
>> propagating from, so we don't turn this:
>>
>> mov(16) vgrf6:F, vgrf7+0.0<0>:F
>> (...)
>> load_payload(8) vgrf28:F, vgrf6+1.0:F 2ndhalf
>> mov(8) vgrf29:DF, vgrf28:F 2ndhalf
>>
>> into:
>>
>> mov(16) vgrf6:F, vgrf7+0.0<0>:F
>> (...)
>> load_payload(8) vgrf28:F, vgrf7+1.0<0>:F 2ndhalf
>> mov(8) vgrf29:DF, vgrf7+1.0<0>:F 2ndhalf
>>
>> and instead we do this:
>>
>> mov(16) vgrf6:F, vgrf7+0.0<0>:F
>> (...)
>> load_payload(8) vgrf28:F, vgrf7+0.0<0>:F 2ndhalf
>> mov(8) vgrf29:DF, vgrf7+0.0<0>:F 2ndhalf
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 16 +++++++++++++---
>>  1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp 
>> b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> index becc8bc..9147e60 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> @@ -460,10 +460,20 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, 
>> acp_entry *entry)
>>            * parts of vgrfs so we have to do some reg_offset magic.
>>            */
>>  
>> -         /* Compute the offset of inst->src[arg] relative to inst->dst */
>> +         /* Compute the offset of inst->src[arg] relative to inst->dst
>> +          *
>> +          * If the source we are copy propagating from has a stride of 0, 
>> then
>> +          * we must not offset into it based on the offset of our source
>> +          * relative to entry->dst
>> +          */
>>           assert(entry->dst.subreg_offset == 0);
>> -         int rel_offset = inst->src[arg].reg_offset - entry->dst.reg_offset;
>> -         int rel_suboffset = inst->src[arg].subreg_offset;
>> +         int rel_offset, rel_suboffset;
>> +         if (entry->src.stride != 0) {
>> +            rel_offset = inst->src[arg].reg_offset - entry->dst.reg_offset;
>> +            rel_suboffset = inst->src[arg].subreg_offset;
>> +         } else {
> 
> Should the comment added above go here instead?
> 
> Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>

Right, I will move it to here.

Thanks,

Sam

> 
>> +            rel_offset = rel_suboffset = 0;
>> +         }
>>  
>>           /* Compute the final register offset (in bytes) */
>>           int offset = entry->src.reg_offset * 32 + entry->src.subreg_offset;
>> -- 
>> 2.5.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to