[Mesa-dev] [PATCH 15/16] i965/fs: Add support for removing MOV.NZ instructions.

[Jason Ekstrand]

On Mon, Jan 19, 2015 at 3:31 PM, Matt Turner <mattst88 at gmail.com> wrote:

> For some reason, we occasionally write the flag register with a MOV.NZ
> instruction:
>
>    add(8)          g25<1>F         -g6<0,1,0>F     g15<8,8,1>F
>    cmp.l.f0(8)     g26<1>D         g25<8,8,1>F     0F
>    mov.nz.f0(8)    null            g26<8,8,1>D
>
> A MOV.NZ instruction on the result of a CMP is like comparing for
> equality with true in C. It's useless. Removing it allows us to
> generate:
>
>    add.l.f0(8)     null            -g6<0,1,0>F     g15<8,8,1>F
>
> total instructions in shared programs: 5955701 -> 5951657 (-0.07%)
> instructions in affected programs:     302910 -> 298866 (-1.34%)
> GAINED:                                1
> LOST:                                  0
> ---
>  .../drivers/dri/i965/brw_fs_cmod_propagation.cpp   | 23 ++++++++++++++--
>  .../drivers/dri/i965/test_fs_cmod_propagation.cpp  | 32
> ++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> index b521350..dd89512 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_cmod_propagation.cpp
> @@ -57,12 +57,20 @@ opt_cmod_propagation_local(fs_visitor *v, bblock_t
> *block)
>     foreach_inst_in_block_reverse_safe(fs_inst, inst, block) {
>        ip--;
>
> -      if (inst->opcode != BRW_OPCODE_CMP ||
> +      if ((inst->opcode != BRW_OPCODE_CMP &&
> +           inst->opcode != BRW_OPCODE_MOV) ||
>            inst->predicate != BRW_PREDICATE_NONE ||
>            !inst->dst.is_null() ||
>            inst->src[0].file != GRF ||
> -          inst->src[0].abs ||
> -          !inst->src[1].is_zero())
> +          inst->src[0].abs)
> +         continue;
> +
> +      if (inst->opcode == BRW_OPCODE_CMP && !inst->src[1].is_zero())
> +         continue;
> +
> +      if (inst->opcode == BRW_OPCODE_MOV &&
> +          (inst->conditional_mod != BRW_CONDITIONAL_NZ ||
> +           inst->src[0].negate))
>

I think negate is ok here.  I'm not 100% sure on the symantics of move.nz,
but if it's a "!= 0" then negation shouldn't matter.  If it only considers
the bottom bit then negation shouldn't matter there either.


>           continue;
>
>        bool read_flag = false;
> @@ -72,6 +80,15 @@ opt_cmod_propagation_local(fs_visitor *v, bblock_t
> *block)
>               scan_inst->dst.reg == inst->src[0].reg &&
>               scan_inst->dst.reg_offset == inst->src[0].reg_offset &&
>               !scan_inst->is_partial_write()) {
> +            if (inst->opcode == BRW_OPCODE_MOV) {
> +               if (!scan_inst->writes_flag())
> +                  break;
> +
> +               inst->remove(block);
> +               progress = true;
> +               break;
> +            }
> +
>              enum brw_conditional_mod cond =
>                 inst->src[0].negate ?
> brw_invert_cmod(inst->conditional_mod)
>                                     : inst->conditional_mod;
> diff --git a/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
> b/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
> index 15f685e..9541597 100644
> --- a/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
> +++ b/src/mesa/drivers/dri/i965/test_fs_cmod_propagation.cpp
> @@ -343,3 +343,35 @@ TEST_F(cmod_propagation_test, negate)
>     EXPECT_EQ(BRW_OPCODE_ADD, instruction(block0, 0)->opcode);
>     EXPECT_EQ(BRW_CONDITIONAL_L, instruction(block0, 0)->conditional_mod);
>  }
> +
> +TEST_F(cmod_propagation_test, movnz)
> +{
> +   fs_reg dest(v, glsl_type::float_type);
> +   fs_reg src0(v, glsl_type::float_type);
> +   fs_reg src1(v, glsl_type::float_type);
> +   v->emit(BRW_OPCODE_CMP, dest, src0, src1)
> +      ->conditional_mod = BRW_CONDITIONAL_GE;
> +   v->emit(BRW_OPCODE_MOV, v->reg_null_f, dest)
> +      ->conditional_mod = BRW_CONDITIONAL_NZ;
> +
> +   /* = Before =
> +    *
> +    * 0: cmp.ge.f0(8)  dest  src0  src1
> +    * 1: mov.nz.f0(8)  null  dest
> +    *
> +    * = After =
> +    * 0: cmp.ge.f0(8)  dest  src0  src1
> +    */
> +
> +   v->calculate_cfg();
> +   bblock_t *block0 = v->cfg->blocks[0];
> +
> +   EXPECT_EQ(0, block0->start_ip);
> +   EXPECT_EQ(1, block0->end_ip);
> +
> +   EXPECT_TRUE(cmod_propagation(v));
> +   EXPECT_EQ(0, block0->start_ip);
> +   EXPECT_EQ(0, block0->end_ip);
> +   EXPECT_EQ(BRW_OPCODE_CMP, instruction(block0, 0)->opcode);
> +   EXPECT_EQ(BRW_CONDITIONAL_GE, instruction(block0, 0)->conditional_mod);
> +}
> --
> 2.0.4
>
> _______________________________________________
> 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/20150120/fe8b8270/attachment.html>