[Mesa-dev] [PATCH 5/7] i965/fs: Less broken handling of force_writemask_all in lower_load_payload().

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

On Sat, Jan 17, 2015 at 6:04 PM, Francisco Jerez <currojerez at riseup.net>
wrote:

> It's perfectly fine to read the second half of a register written with
> force_writemask_all from a first half MOV instruction or vice versa, and
> lower_load_payload shouldn't mark the whole MOV as belonging to the second
> half in that case.  Replicate the same metadata to both halves of the
> destination when writemasking is disabled.
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 4a61943..d585a67 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3059,9 +3059,11 @@ fs_visitor::lower_load_payload()
>        }
>
>        if (inst->dst.file == MRF || inst->dst.file == GRF) {
> -         bool force_sechalf = inst->force_sechalf;
> +         bool force_sechalf = inst->force_sechalf &&
> +                              !inst->force_writemask_all;
>           bool toggle_sechalf = inst->dst.width == 16 &&
> -                               type_sz(inst->dst.type) == 4;
> +                               type_sz(inst->dst.type) == 4 &&
> +                               !inst->force_writemask_all;
>           for (int i = 0; i < inst->regs_written; ++i) {
>              metadata[dst_reg + i].written = true;
>              metadata[dst_reg + i].force_sechalf = force_sechalf;
> @@ -3104,11 +3106,15 @@ fs_visitor::lower_load_payload()
>                    mov->force_writemask_all =
> metadata[src_reg].force_writemask_all;
>                    metadata[dst_reg] = metadata[src_reg];
>                    if (dst.width * type_sz(dst.type) > 32) {
> -                     assert((!metadata[src_reg].written ||
> -                             !metadata[src_reg].force_sechalf) &&
> -                            (!metadata[src_reg + 1].written ||
> -                             metadata[src_reg + 1].force_sechalf));
> -                     metadata[dst_reg + 1] = metadata[src_reg + 1];
> +                     if (metadata[src_reg].force_writemask_all) {
> +                        metadata[dst_reg + 1] = metadata[src_reg];
> +                     } else {
> +                        assert((!metadata[src_reg].written ||
> +                                !metadata[src_reg].force_sechalf) &&
> +                               (!metadata[src_reg + 1].written ||
> +                                metadata[src_reg + 1].force_sechalf));
> +                        metadata[dst_reg + 1] = metadata[src_reg + 1];
> +                     }
>                    }
>                 } else {
>                    metadata[dst_reg].force_writemask_all = false;
> --
> 2.1.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150220/f33c1ca9/attachment.html>