<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, Jan 20, 2015 at 3:31 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 3:17 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > > On Tue, Jan 20, 2015 at 3:09 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote: >> >> 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. >> >> 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... </div></div>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. </blockquote></div> </div><div class="gmail_extra">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. </div></div>