[Mesa-dev] [PATCH] i965/fs: Fix off-by-one region overlap comparison in copy propagation.

[Samuel Iglesias Gonsálvez]

Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>

On 24/05/16 05:34, Francisco Jerez wrote:
> This was introduced in cf375a3333e54a01462f192202d609436e5fbec8 but
> the blame is mine because the pseudocode I sent in my review comment
> for the original patch suggesting to do things this way already had
> the off-by-one error.  This may have caused copy propagation to be
> unnecessarily strict while checking whether VGRF writes interfere with
> any ACP entries and possibly miss valid optimization opportunities in
> cases where multiple copy instructions write sequential locations of
> the same VGRF.
> 
> Cc: Iago Toral Quiroga <itoral at igalia.com>
> Cc: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 4 ++--
>  1 file changed, 2 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 b667b8c..eb5c00d 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -756,8 +756,8 @@ inline bool
>  regions_overlap(const fs_reg &r, unsigned n, const fs_reg &s, unsigned m)
>  {
>     return r.file == s.file && r.nr == s.nr &&
> -      !(r.reg_offset + n < s.reg_offset ||
> -        s.reg_offset + m < r.reg_offset);
> +      !(r.reg_offset + n <= s.reg_offset ||
> +        s.reg_offset + m <= r.reg_offset);
>  }
>  
>  /* Walks a basic block and does copy propagation on it using the acp
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160524/ee641679/attachment.sig>