[Mesa-dev] [PATCH 5/9] i965: Combine assembly annotations if possible.

[Kenneth Graunke]

On Tuesday, November 03, 2015 01:23:13 PM Matt Turner wrote:
> On Mon, Oct 26, 2015 at 5:08 AM, Pohjolainen, Topi
> <topi.pohjolainen at intel.com> wrote:
> > On Wed, Oct 21, 2015 at 03:58:13PM -0700, Matt Turner wrote:
> >> Often annotations are identical between sets of consecutive
> >> instructions. We can perhaps avoid some memory allocations by reusing
> >> the previous annotation.
> >> ---
> >>  src/mesa/drivers/dri/i965/intel_asm_annotation.c | 19 ++++++++++++++-----
> >>  1 file changed, 14 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/intel_asm_annotation.c b/src/mesa/drivers/dri/i965/intel_asm_annotation.c
> >> index f87a9bb..58830db 100644
> >> --- a/src/mesa/drivers/dri/i965/intel_asm_annotation.c
> >> +++ b/src/mesa/drivers/dri/i965/intel_asm_annotation.c
> >> @@ -112,6 +112,20 @@ void annotate(const struct brw_device_info *devinfo,
> >>        ann->block_start = cfg->blocks[annotation->cur_block];
> >>     }
> >>
> >> +   if (bblock_end(cfg->blocks[annotation->cur_block]) == inst) {
> >> +      ann->block_end = cfg->blocks[annotation->cur_block];
> >> +      annotation->cur_block++;
> >> +   }
> >> +
> >> +   /* Merge this annotation with the previous if possible. */
> >> +   struct annotation *prev = &annotation->ann[annotation->ann_count - 2];
> >
> > What guarantees that annotation->ann_count is always at least two at this
> > point?
> 
> Nothing! Good catch :)
> 
> I'll add "annotation->ann_count >= 2 &&" as the first part of the
> conditional immediately below.

I suppose that works, but having a 'prev' pointer around that points to
out-of-bounds memory is a bit ugly...

How about:

   struct annotation *prev = annotation->ann_count > 1 ?
      &annotation->ann[annotation->ann_count - 2] : NULL;

   if (prev &&
       ann->ir == prev->ir &&
       ...)

With that change,
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> >> +   if (ann->ir == prev->ir &&
> >> +       ann->annotation == prev->annotation &&
> >> +       ann->block_start == NULL) {
> >> +      annotation->ann_count--;
> >> +      return;
> >> +   }
> >> +
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151103/f8c8f777/attachment.sig>