<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Jan 20, 2015 at 3:09 PM, Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Jan 20, 2015 at 2:58 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > On Mon, Jan 19, 2015 at 3:31 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote: >> >> For some reason, we occasionally write the flag register with a <a href="http://MOV.NZ" target="_blank">MOV.NZ</a> >> 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 <a href="http://MOV.NZ" target="_blank">MOV.NZ</a> 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 <a href="http://move.nz" target="_blank">move.nz</a>, > 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. </div></div>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. </blockquote></div> </div><div class="gmail_extra">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... </div><div class="gmail_extra"> </div></div>