[Mesa-dev] [PATCH 2/2] i965: Don't copy propagate bitcasts with source modifiers.

[Paul Berry]

On 12 August 2013 13:18, Matt Turner <mattst88 at gmail.com> wrote:

> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp                    | 13 +++++++++++++
>  src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp   |  3 +++
>  src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 10 ++++++----
>  3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index a81e97f..f104f8c 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -2118,6 +2118,19 @@ fs_visitor::register_coalesce()
>             }
>          }
>
> +         if (has_source_modifiers) {
> +            for (int i = 0; i < 3; i++) {
> +               if (scan_inst->src[i].file == GRF &&
> +                   scan_inst->src[i].reg == inst->dst.reg &&
> +                   scan_inst->src[i].reg_offset == inst->dst.reg_offset &&
> +                   inst->dst.type != scan_inst->src[i].type)
> +               {
> +                 return false;
> +               }
> +            }
> +         }
> +
> +
>

I don't understand the whole patch, but I'm pretty sure it's wrong to
return false in this case, because that will exit the entire
fs_visitor::register_coalesce() function.


>          /* The gen6 MATH instruction can't handle source modifiers or
>           * unusual register regions, so avoid coalescing those for
>           * now.  We should do something more specific.
> 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 234f8bd..a5cd858 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
> @@ -221,6 +221,9 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg,
> acp_entry *entry)
>          entry->src.smear != -1) && !can_do_source_mods(inst))
>        return false;
>
> +   if (has_source_modifiers && (entry->dst.type != inst->src[arg].type))
> +      return false;
> +
>     inst->src[arg].file = entry->src.file;
>     inst->src[arg].reg = entry->src.reg;
>     inst->src[arg].reg_offset = entry->src.reg_offset;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> index c28d0de..2a2d403 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
> @@ -206,14 +206,16 @@ vec4_visitor::try_copy_propagation(vec4_instruction
> *inst, int arg,
>     if (inst->src[arg].negate)
>        value.negate = !value.negate;
>
> -   bool has_source_modifiers = (value.negate || value.abs ||
> -                                value.swizzle != BRW_SWIZZLE_XYZW ||
> -                                value.file == UNIFORM);
> +   bool has_source_modifiers = value.negate || value.abs;
>
>     /* gen6 math and gen7+ SENDs from GRFs ignore source modifiers on
>      * instructions.
>      */
> -   if (has_source_modifiers && !can_do_source_mods(inst))
> +   if ((has_source_modifiers || value.file == UNIFORM ||
> +        value.swizzle != BRW_SWIZZLE_XYZW) && !can_do_source_mods(inst))
> +      return false;
> +
> +   if (has_source_modifiers && (value.type != inst->src[arg].type))
>        return false;
>
>     bool is_3src_inst = (inst->opcode == BRW_OPCODE_LRP ||
> --
> 1.8.3.2
>
> _______________________________________________
> 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/20130815/f3ff14d2/attachment.html>