[Mesa-dev] [PATCH 04/23] i965/fs: fix requirements to allow type change in copy-propagation

[Samuel Iglesias Gonsálvez]

On 04/05/16 01:04, Jordan Justen wrote:
> Should the subject say something about not allowing a type size
> change?
> 

I don't think it is necessary but I don't have a strong opinion. What do
you think about this?

i965/fs: don't allow type change in copy-propagation if types have
different sizes

Sam

> -Jordan
> 
> On 2016-05-03 05:21:53, Samuel Iglesias Gonsálvez wrote:
>> From: Iago Toral Quiroga <itoral at igalia.com>
>>
>> When source modifiers are present and the types of the source and
>> the entry's source are different, there are certain cases in which
>> we allow copy-propagation to change the type of source by the type
>> of the entry's source we are copy propagating from.
>>
>> However, it is not generally safe to do this if the types involved
>> have different sizes, since parameters like the stride would change
>> the semantics of the resulting instruction.
>>
>> Prevents that we turn this:
>>
>> load_payload(8) vgrf17:DF, |vgrf4+0.0|:DF 1sthalf
>> mov(8) vgrf18:DF, vgrf17:DF 1sthalf
>> load_payload(8) vgrf5:DF, vgrf18:DF, vgrf20:DF NoMask 1sthalf WE_all
>> load_payload(8) vgrf21:UD, vgrf5+0.4<2>:UD 1sthalf
>> mov(8) vgrf22:UD, vgrf21:UD 1sthalf
>>
>> into:
>>
>> load_payload(8) vgrf17:DF, |vgrf4+0.0|:DF 1sthalf
>> mov(8) vgrf18:DF, |vgrf4+0.0|:DF 1sthalf
>> load_payload(8) vgrf5:DF, |vgrf4+0.0|:DF, |vgrf4+2.0|:DF NoMask 1sthalf WE_all
>> load_payload(8) vgrf21:UD, vgrf5+0.4<2>:UD 1sthalf
>> mov(8) vgrf22:DF, |vgrf4+0.4|<2>:DF 1sthalf
>>
>> where the semantics of the last instruccion have changed.
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 5 +++--
>>  1 file changed, 3 insertions(+), 2 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 abc68c8..aa4c9c9 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> @@ -411,8 +411,9 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)
>>        return false;
>>  
>>     if (has_source_modifiers &&
>> -       entry->dst.type != inst->src[arg].type &&
>> -       !inst->can_change_types())
>> +       ((entry->dst.type != inst->src[arg].type &&
>> +         !inst->can_change_types()) ||
>> +        (type_sz(entry->dst.type) != type_sz(inst->src[arg].type))))
>>        return false;
>>  
>>     if (devinfo->gen >= 8 && (entry->src.negate || entry->src.abs) &&
>> -- 
>> 2.5.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>