[Mesa-dev] [PATCH 2/5] intel/fs: Lower integer multiply correctly when destination stride equals 4.

[Francisco Jerez]

Francisco Jerez <currojerez at riseup.net> writes:

> Because the "low" temporary needs to be accessed with word type and
> twice the original stride, attempting to preserve the alignment of the
> original destination can potentially lead to instructions with illegal
> destination stride greater than four.  Because the CHV/BXT alignment
> restrictions are now being enforced by the regioning lowering pass run
> after lower_integer_multiplication(), there is no real need to
> preserve the original strides anymore.
>
> Note that this bug can be reproduced on stable branches, but
> back-porting would be non-trivial, because the fix relies on the
> regioning lowering pass recently introduced.

This and PATCH 3 of this series should have landed with a:

Cc: 19.0 <mesa-stable at lists.freedesktop.org>
    
The paragraph above recommending against their inclusion in stable
branches was referring to the *then* stable branches when the patch was
written, a couple of weeks before the 19.0 branch point.  Unfortunately
the series landed after the branchpoint and patches 2 and 3 missed the
19.0 branch they were intended for.  Any chance we could get these into
the next 19.0 stable release?

Thanks to Tapani for realizing the problem!

> ---
>  src/intel/compiler/brw_fs.cpp | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index f475b617df2..5768e0d6542 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -3962,13 +3962,11 @@ fs_visitor::lower_integer_multiplication()
>                  regions_overlap(inst->dst, inst->size_written,
>                                  inst->src[0], inst->size_read(0)) ||
>                  regions_overlap(inst->dst, inst->size_written,
> -                                inst->src[1], inst->size_read(1))) {
> +                                inst->src[1], inst->size_read(1)) ||
> +                inst->dst.stride >= 4) {
>                 needs_mov = true;
> -               /* Get a new VGRF but keep the same stride as inst->dst */
>                 low = fs_reg(VGRF, alloc.allocate(regs_written(inst)),
>                              inst->dst.type);
> -               low.stride = inst->dst.stride;
> -               low.offset = inst->dst.offset % REG_SIZE;
>              }
>  
>              /* Get a new VGRF but keep the same stride as inst->dst */
> -- 
> 2.19.2
>
> _______________________________________________
> 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: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190429/d33760b2/attachment.sig>