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

[Jason Ekstrand]

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

> On Tue, Jan 20, 2015 at 3:17 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Tue, Jan 20, 2015 at 3:09 PM, Matt Turner <mattst88 at gmail.com> wrote:
> >>
> >> On Tue, Jan 20, 2015 at 2:58 PM, Jason Ekstrand <jason at jlekstrand.net>
> >> wrote:
> >> > 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.
> >>
> >> The instruction "mov.nz.f0 null src0" sets f0 if src0 != 0.
> >>
> >> Hmm, you're right. Since we're only allowing NZ conditional modifiers
> >> we can also allow negation. I don't think we'll ever generate that,
> >> but okay. I'll remove the inst->src[0].negate check.
> >
> >
> > Sure we will.  When we do older gens in NIR, we'll emit one of those
> after
> > every cmp.  Still have to deal with the and though...
>
> Emitting it after every comparison isn't what you want. We emit it
> from resolve_bool_comparison() before we need the integer
> representation of a bool for things like b2f. NIR -> FS should behave
> the same way.
>

Except typeless...  We need some sort of assurance that the result of a NIR
comparison is always 0 or ~0.  We may be able to pull similar stunts or
maybe do the cleanup in NIR, but I'm not sure if we can or not.  It's going
to be a bit interesting.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150120/b8a4137d/attachment.html>