Samuel Iglesias Gonsálvez <sigles...@igalia.com> writes:

> From: Iago Toral Quiroga <ito...@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 <curroje...@riseup.net>

>              }
>  
>              lbld.emit(split_inst);
> -- 
> 2.5.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Attachment: signature.asc
Description: PGP signature

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to