Re: [Mesa-dev] [PATCH 6/6] i965/fs: don't copy propagate if the instruction writes to more than two adjacent GRFs

[Samuel Iglesias Gonsálvez]

On 08/07/16 04:49, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez  writes:
> 
>> This is not allowed by the HW and copy propagation can hide this issue to
>> lower_simd_width pass, which is going to fix it.
>>
>> Signed-off-by: Samuel Iglesias Gonsálvez 
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> 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 438f681..c7f7628 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> @@ -752,6 +752,7 @@ can_propagate_from(fs_inst *inst)
>>  inst->src[0].file == UNIFORM ||
>>  inst->src[0].file == IMM) &&
>> inst->src[0].type == inst->dst.type &&
>> +   inst->regs_written <= 2 &&
> 
> This doesn't look right to me, why should copy propagation care whether
> the SIMD width of a MOV instruction it's going to propagate away is
> allowed by the hardware or not?  The "illegal" copy instruction is
> already there anyway, and preventing copy propagation from doing its job
> in that case can only increase the likelihood that the unsupported
> instruction will remain in the program which implies more work for the
> SIMD lowering pass at a later point.
> 

Right, I understood wrongly the issue. Today I found what was going-on
and I will send a patch as part of the v2 of the series.

Please ignore this patch.

Thanks,

Sam


>> !inst->is_partial_write());
>>  }
>>  
>> -- 
>> 2.7.4
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev



signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/6] i965/fs: don't copy propagate if the instruction writes to more than two adjacent GRFs

[Francisco Jerez]

Samuel Iglesias Gonsálvez  writes:

> This is not allowed by the HW and copy propagation can hide this issue to
> lower_simd_width pass, which is going to fix it.
>
> Signed-off-by: Samuel Iglesias Gonsálvez 
> ---
>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 1 +
>  1 file changed, 1 insertion(+)
>
> 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 438f681..c7f7628 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -752,6 +752,7 @@ can_propagate_from(fs_inst *inst)
>  inst->src[0].file == UNIFORM ||
>  inst->src[0].file == IMM) &&
> inst->src[0].type == inst->dst.type &&
> +   inst->regs_written <= 2 &&

This doesn't look right to me, why should copy propagation care whether
the SIMD width of a MOV instruction it's going to propagate away is
allowed by the hardware or not?  The "illegal" copy instruction is
already there anyway, and preventing copy propagation from doing its job
in that case can only increase the likelihood that the unsupported
instruction will remain in the program which implies more work for the
SIMD lowering pass at a later point.

> !inst->is_partial_write());
>  }
>  
> -- 
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 6/6] i965/fs: don't copy propagate if the instruction writes to more than two adjacent GRFs

[Samuel Iglesias Gonsálvez]

This is not allowed by the HW and copy propagation can hide this issue to
lower_simd_width pass, which is going to fix it.

Signed-off-by: Samuel Iglesias Gonsálvez 
---
 src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 1 +
 1 file changed, 1 insertion(+)

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 438f681..c7f7628 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
@@ -752,6 +752,7 @@ can_propagate_from(fs_inst *inst)
 inst->src[0].file == UNIFORM ||
 inst->src[0].file == IMM) &&
inst->src[0].type == inst->dst.type &&
+   inst->regs_written <= 2 &&
!inst->is_partial_write());
 }
 
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev