<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Sep 23, 2014 at 1:28 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"><span class="">On Tue, Sep 23, 2014 at 1:19 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> 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>
</span><div><div class="h5">>>     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 to<br>
>> be<br>
>> -          * aligned, or the GPU hangs.<br>
>> +          * aligned, or the GPU hangs. All uncompacted instructions 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>
</div></div>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>
</blockquote></div><br></div><div class="gmail_extra">Why didn't we need that post-gen5 and, if we didn't, why are we doing it unconditionally?<br></div></div>