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

[Matt Turner]

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.