[Mesa-dev] [PATCH v3 15/24] i965/vec4: fix VEC4_OPCODE_FROM_DOUBLE for IVB/BYT

[Francisco Jerez]

Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:

> From: "Juan A. Suarez Romero" <jasuarez at igalia.com>
>
> In the generator we must generate slightly different code for
> Ivybridge/Baytrail, because of the way the stride works in
> this hardware.
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 7bb1ab1879c..7fa1afc9073 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1948,13 +1948,28 @@ generate_code(struct brw_codegen *p,
>  
>           brw_set_default_access_mode(p, BRW_ALIGN_1);
>  
> -         dst.hstride = BRW_HORIZONTAL_STRIDE_2;

How would this know whether there was enough space allocated in the
destination to hold the destination value with a stride of two?  What if
the following SIMD vector in the same GRF allocation actually contained
useful single-precision data that is getting corrupted by the oversized
strided destination?  I think we should be doing this pre-regalloc so we
can allocate a temporary large enough to hold the strided value...

> +         /* When converting from DF->F, we set destination's stride as 2 as an
> +          * aligment requirement. But in IVB/BYT, each DF implicitly writes
> +          * two floats, being the first one the converted value. So we don't
> +          * need to explicitly set stride 2, but 1.
> +          */
> +         if (devinfo->gen == 7 && !devinfo->is_haswell)
> +            dst.hstride = BRW_HORIZONTAL_STRIDE_1;
> +         else
> +            dst.hstride = BRW_HORIZONTAL_STRIDE_2;
> +
>           dst.width = BRW_WIDTH_4;
>           src[0].vstride = BRW_VERTICAL_STRIDE_4;
>           src[0].width = BRW_WIDTH_4;
>           brw_MOV(p, dst, src[0]);
>  
spread(dst, desired-stride) so you don't mess up the original
destination brw_reg and have to fix it up again later.

>           struct brw_reg dst_as_src = dst;
> +         /* As we have set horizontal stride 1 instead of 2 in IVB/BYT, we
> +          * need to fix it here to have the expected value.
> +          */
> +         if (devinfo->gen == 7 && !devinfo->is_haswell)
> +            dst_as_src.hstride = BRW_HORIZONTAL_STRIDE_2;
> +
>           dst.hstride = BRW_HORIZONTAL_STRIDE_1;
>           dst.width = BRW_WIDTH_8;
>           brw_MOV(p, dst, dst_as_src);
> -- 
> 2.11.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: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170303/f2cbf901/attachment.sig>