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

[Francisco Jerez]

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...

>  
>           /* 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-i965-fs-Fix-propagation-of-copies-with-strided-sourc.patch
Type: text/x-diff
Size: 2975 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160510/303c4b2d/attachment.patch>
-------------- 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/20160510/303c4b2d/attachment.sig>