[Mesa-dev] [PATCH 11/14] i965/compaction: Add support for G45.

[Jason Ekstrand]

On Tue, Sep 23, 2014 at 2:21 PM, Matt Turner <mattst88 at gmail.com> wrote:

> On Tue, Sep 23, 2014 at 2:08 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > On Tue, Sep 23, 2014 at 1:28 PM, Matt Turner <mattst88 at gmail.com> wrote:
> >>
> >> On Tue, Sep 23, 2014 at 1:19 PM, Jason Ekstrand <jason at jlekstrand.net>
> >> wrote:
> >> > On Thu, Aug 28, 2014 at 8:10 PM, Matt Turner <mattst88 at gmail.com>
> wrote:
> >> >>     int offset = 0;
> >> >> @@ -1319,17 +1323,22 @@ brw_compact_instructions(struct brw_compile
> *p,
> >> >> int start_offset,
> >> >>           offset += sizeof(brw_compact_inst);
> >> >>        } else {
> >> >>           /* It appears that the end of thread SEND instruction needs
> >> >> to
> >> >> be
> >> >> -          * aligned, or the GPU hangs.
> >> >> +          * aligned, or the GPU hangs. All uncompacted instructions
> >> >> need
> >> >> to be
> >> >> +          * aligned on G45.
> >> >>            */
> >> >> -         if ((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||
> >> >> -              brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&
> >> >> -             brw_inst_eot(brw, src) &&
> >> >> -             (offset & sizeof(brw_compact_inst)) != 0) {
> >> >> +         if ((offset & sizeof(brw_compact_inst)) != 0 &&
> >> >> +             (((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||
> >> >> +                brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&
> >> >> +               brw_inst_eot(brw, src)) ||
> >> >> +              brw->is_g4x)) {
> >> >>              brw_compact_inst *align = store + offset;
> >> >>              memset(align, 0, sizeof(*align));
> >> >> -            brw_compact_inst_set_opcode(align, BRW_OPCODE_NOP);
> >> >> +            brw_compact_inst_set_opcode(align, brw->is_g4x ?
> >> >> BRW_OPCODE_NENOP :
> >> >> +
> >> >> BRW_OPCODE_NOP);
> >> >>              brw_compact_inst_set_cmpt_control(align, true);
> >> >>              offset += sizeof(brw_compact_inst);
> >> >> +            compacted_count--;
> >> >> +            compacted_counts[src_offset / sizeof(brw_inst)] =
> >> >> compacted_count;
> >> >
> >> >
> >> > Do these two lines really belong in this patch?  They seem completely
> >> > unrelated to stuff on G45.
> >>
> >> Yes, because if you have to insert a padding NENOP, you need to update
> >> the compaction_count (which is really a metric of how far offset you
> >> are because of the space saved by compacted instructions).
> >>
> >> So, really what it's doing is not considering compacted instructions
> >> as saving space if we had to insert a padding NENOP after it.
> >
> >
> > Why didn't we need that post-gen5 and, if we didn't, why are we doing it
> > unconditionally?
>
> We don't need it post-Gen5 because those platforms jump instructions
> can jump to 64-bit aligned instructions, while Gen4 can't.
>
> It's not unconditional -- it happens only when we have to insert a
> padding instruction:
>

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
--Jason


>
> > +         if ((offset & sizeof(brw_compact_inst)) != 0 &&
> > +             (((brw_inst_opcode(brw, src) == BRW_OPCODE_SEND ||
> > +                brw_inst_opcode(brw, src) == BRW_OPCODE_SENDC) &&
> > +               brw_inst_eot(brw, src)) ||
> > +              brw->is_g4x)) {
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140923/567f150a/attachment-0001.html>