<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 23, 2014 at 2:21 PM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><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, Sep 23, 2014 at 2:08 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> On Tue, Sep 23, 2014 at 1:28 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>><br>
>> On Tue, Sep 23, 2014 at 1:19 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
>> wrote:<br>
>> > On Thu, Aug 28, 2014 at 8:10 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>> >>     int offset = 0;<br>
>> >> @@ -1319,17 +1323,22 @@ brw_compact_instructions(struct brw_compile *p,<br>
>> >> int start_offset,<br>
>> >>           offset += sizeof(brw_compact_inst);<br>
>> >>        } else {<br>
>> >>           /* It appears that the end of thread SEND instruction needs<br>
>> >> to<br>
>> >> be<br>
>> >> -          * aligned, or the GPU hangs.<br>
>> >> +          * aligned, or the GPU hangs. All uncompacted instructions<br>
>> >> need<br>
>> >> to be<br>
>> >> +          * aligned on G45.<br>
>> >>            */<br>
>> >> -         if ((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||<br>
>> >> -              brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&<br>
>> >> -             brw_inst_eot(brw, src) &&<br>
>> >> -             (offset & sizeof(brw_compact_inst)) != 0) {<br>
>> >> +         if ((offset & sizeof(brw_compact_inst)) != 0 &&<br>
>> >> +             (((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||<br>
>> >> +                brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&<br>
>> >> +               brw_inst_eot(brw, src)) ||<br>
>> >> +              brw->is_g4x)) {<br>
>> >>              brw_compact_inst *align = store + offset;<br>
>> >>              memset(align, 0, sizeof(*align));<br>
>> >> -            brw_compact_inst_set_opcode(align, BRW_OPCODE_NOP);<br>
>> >> +            brw_compact_inst_set_opcode(align, brw->is_g4x ?<br>
>> >> BRW_OPCODE_NENOP :<br>
>> >> +<br>
>> >> BRW_OPCODE_NOP);<br>
>> >>              brw_compact_inst_set_cmpt_control(align, true);<br>
>> >>              offset += sizeof(brw_compact_inst);<br>
>> >> +            compacted_count--;<br>
>> >> +            compacted_counts[src_offset / sizeof(brw_inst)] =<br>
>> >> compacted_count;<br>
>> ><br>
>> ><br>
>> > Do these two lines really belong in this patch?  They seem completely<br>
>> > unrelated to stuff on G45.<br>
>><br>
>> Yes, because if you have to insert a padding NENOP, you need to update<br>
>> the compaction_count (which is really a metric of how far offset you<br>
>> are because of the space saved by compacted instructions).<br>
>><br>
>> So, really what it's doing is not considering compacted instructions<br>
>> as saving space if we had to insert a padding NENOP after it.<br>
><br>
><br>
> Why didn't we need that post-gen5 and, if we didn't, why are we doing it<br>
> unconditionally?<br>
<br>
</div></div>We don't need it post-Gen5 because those platforms jump instructions<br>
can jump to 64-bit aligned instructions, while Gen4 can't.<br>
<br>
It's not unconditional -- it happens only when we have to insert a<br>
padding instruction:<br></blockquote><div><br></div><div>Ok, I missed the fact that we only did before on EOT.  I guess it makes sense now.  Go ahead and stick my RB on it<br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> +         if ((offset & sizeof(brw_compact_inst)) != 0 &&<br>
> +             (((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||<br>
> +                brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&<br>
> +               brw_inst_eot(brw, src)) ||<br>
> +              brw->is_g4x)) {<br>
</div></div></blockquote></div><br></div></div>