[Mesa-dev] [PATCH 02/23] i965/fs: fix copy propagation from sources with stride 0

[Samuel Iglesias Gonsálvez]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256



On 10/05/16 22:57, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
> 
>> From: Iago Toral Quiroga <itoral at 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 { +
>> rel_offset = rel_suboffset = 0; +         }
> 
> Sigh, this fixes the problem for stride == 0 but leaves the logic
> broken in the general case.  Turns out I came across the same
> problem with SIMD32 and came up with the fix below that should work
> regardless of the stride value...
> 

Thanks for the fix! We will use it instead and test it.

Sam

>> 
>> /* 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 at lists.freedesktop.org 
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXMti6AAoJEH/0ujLxfcNDsv4P/3EnZMjaOWthqw6A1i6IlT2C
Z9nZlteua2V7guqC/yFIZQeX6UffSP9BzQ0ZUiCG/+9jg6zBfSzDNCEgWkLu/CxA
Ihc8mcFh82Hxb8c6BVgduE2Ir08o34cYHMeojlVOwQVPavj3X/pbaeiYTyizriI4
NK+VjyfYBeOcfCN0uxSpOkhU+QMSN4XTJS7hly3+KmIWcY8ocR6Ymy+Dr1KFpn2u
U0EdeQZ8HLKPhufMT3sTRx6sazBl39OeBfSLF6s2Gfc8mXX3obS7F4A+HIfXUqIg
KJAt6fYU7aMSaxzx/kcjQ0WuZV+LJvTn/tgFPrZoBpXlwAdvVqPX2hiu+wUfOM6u
nibSu9p5DdP5wxWMWPJSn8BNAcOW/eBrzLBd8jCYEnhANdStwQTC2gukI/M5pN81
Kdid/T4BfWHeAPJsK6N9+oxix+ecrs6VXwXVuraPgBwBG5LYYm8diYJ5tImnL7kh
AcvlHywmJw8Y8ocnoQ+DXVb3amzygWctXrFmTQ1SW9ddLXK5YPrPrR9s5RtPpopl
I4bFCKLRGWQpSxcMdu0OAMn+VaaToOEZHMar1NA+qxCT7hziSjGz33ypJBN+jW3P
IUYGsNKcBX3Lq75u/C0y7IOc8q+YMm3kUIbPET05t4Zd7QeNP1WuOEepoB4yXe3Y
zkDenIskGfWfU/l587kJ
=GihL
-----END PGP SIGNATURE-----