[Mesa-dev] [PATCH v2 03/30] i965/fs: Fix copy propagation of load payload for double operands

Francisco Jerez currojerez at riseup.net
Thu May 12 23:52:51 UTC 2016


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

> From: Iago Toral Quiroga <itoral at igalia.com>
>
> Specifically, consider the size of the data type of the operand to compute
> the number of registers written.
>
> v2 (Sam):
> - Fix line width (Jordan).
> - Add an assert (Jordan).
> - Use REG_SIZE in the calculation of regs_written (Curro)
>
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
> ---
>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> 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 83791bf..fd423a3 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -774,8 +774,10 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, bblock_t *block,
>                   inst->dst.file == VGRF) {
>           int offset = 0;
>           for (int i = 0; i < inst->sources; i++) {
> +            assert(type_sz(inst->src[i].type) >= 4);

The assertion doesn't look right to me, if you had taken my suggestion
into account the code below wouldn't care what the type size is as long
as:

| assert(effective_width * type_sz(...) % REG_SIZE == 0);

>              int effective_width = i < inst->header_size ? 8 : inst->exec_size;
> -            int regs_written = effective_width / 8;
> +            int regs_written = effective_width / 8 *
> +               type_sz(inst->src[i].type) / REG_SIZE;

Sigh, you left the '/ 8' in there, the expression should have been
'effective_width * type_sz(inst->src[i].type) / REG_SIZE'.

>              if (inst->src[i].file == VGRF) {
>                 acp_entry *entry = ralloc(copy_prop_ctx, acp_entry);
>                 entry->dst = inst->dst;
> -- 
> 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/20160512/5dda5497/attachment.sig>


More information about the mesa-dev mailing list