[Mesa-dev] [PATCH 2/6] i965/fs: Recognize nop-MOV instructions early.

[Kenneth Graunke]

On 04/16/2014 11:07 AM, Matt Turner wrote:
> And avoid rewriting other instructions unnecessarily. Removes a few
> self-moves we weren't able to handle because they were components of a
> large VGRF.
> 
> instructions in affected programs:     830 -> 826 (-0.48%)
> ---
>  src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
> index f6d9b68..8b37ed0 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_register_coalesce.cpp
> @@ -44,6 +44,12 @@
>  #include "brw_fs_live_variables.h"
>  
>  static bool
> +is_nop_mov(const fs_inst *inst)
> +{
> +   return inst->dst.equals(inst->src[0]);

It feels weird having a function called is_nop_mov that doesn't ensure
that inst->opcode == BRW_OPCODE_MOV.  I know it's unnecessary, since
this function is only called after is_coalesce_candidate(), but...it
might be nice to add that check anyway, or maybe a comment or assertion.

> +}
> +
> +static bool
>  is_coalesce_candidate(const fs_inst *inst, const int *virtual_grf_sizes)
>  {
>     if (inst->opcode != BRW_OPCODE_MOV ||
> @@ -70,9 +76,7 @@ can_coalesce_vars(brw::fs_live_variables *live_intervals,
>                    const exec_list *instructions, const fs_inst *inst,
>                    int var_to, int var_from)
>  {
> -   if (live_intervals->vars_interfere(var_from, var_to) &&
> -       !inst->dst.equals(inst->src[0])) {
> -
> +   if (live_intervals->vars_interfere(var_from, var_to)) {
>        /* We know that the live ranges of A (var_from) and B (var_to)
>         * interfere because of the ->vars_interfere() call above. If the end
>         * of B's live range is after the end of A's range, then we know two
> @@ -131,6 +135,12 @@ fs_visitor::register_coalesce()
>        if (!is_coalesce_candidate(inst, virtual_grf_sizes))
>           continue;
>  
> +      if (is_nop_mov(inst)) {
> +         inst->opcode = BRW_OPCODE_NOP;
> +         progress = true;
> +         continue;
> +      }

Having the can_coalesce_vars function alter the instruction stream also
seems strange.  What do you think about moving the MOV->NOP code into
the caller?  It would be outside the vars_interfere path, but I think
that's okay...

> +
>        if (reg_from != inst->src[0].reg) {
>           reg_from = inst->src[0].reg;
>  
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140417/6d724951/attachment.sig>