[Mesa-dev] [PATCH 54/59] i965/fs/lower_simd_width: Fix registers written for split instructions

[Francisco Jerez]

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

> From: Iago Toral Quiroga <itoral at igalia.com>
>
> When the original instruction had a stride > 1, the combined registers
> written by the split instructions won't amount to the same register space
> written by the original instruction because the split instructions will
> use a stride of 1. The current code assumed otherwise and computed the
> number of registers written by split instructions as an equal share based
> on the relation between the lowered width and the original execution size
> of the instruction.
>
> It is only after the split, when we interleave the components of the result
> from the lowered instructions back into the original dst register, that the
> original stride takes effect and we write all the registers specified by
> the original instruction.
>
> Just make the number of register written the same as the vgrf space we
> allocate for the dst of the split instruction.
>
> Fixes crashes in fp64 tests produced as a result of assigning incorrectly the
> number of registers written by split instructions, which led to incorrect
> validation of the size of the writes against the allocated vgrf space.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index fbdd802..a5caafb 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -4709,8 +4709,8 @@ fs_visitor::lower_simd_width()
>                 split_inst.dst = dsts[i] =
>                    lbld.vgrf(inst->dst.type, dst_size);
>                 split_inst.regs_written =
> -                  DIV_ROUND_UP(inst->regs_written * lower_width,
> -                               inst->exec_size);
> +                  DIV_ROUND_UP(type_sz(inst->dst.type) * dst_size * lower_width,
> +                               REG_SIZE);

LGTM -- Though if you used the component_size() helper instead of
'type_sz(...) * lower_width' you'd make sure things still work if we
have to preserve the original strides at some point in the future.
Either way the patch is:

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

>              }
>  
>              lbld.emit(split_inst);
> -- 
> 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: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160429/7c8c9d46/attachment.sig>