[Mesa-dev] [PATCH 8/8] i965: Remove redundant discard jumps.

[Kenneth Graunke]

On Tuesday, February 24, 2015 12:20:08 PM Eric Anholt wrote:
> Kenneth Graunke <kenneth at whitecape.org> writes:
> 
> > With the previous optimization in place, some shaders wind up with
> > multiple discard jumps in a row, or jumps directly to the next
> > instruction.  We can remove those.
> >
> > Without NIR on Haswell:
> > total instructions in shared programs: 5777258 -> 5775872 (-0.02%)
> > instructions in affected programs:     20312 -> 18926 (-6.82%)
> > helped:                                716
> >
> > With NIR on Haswell:
> > total instructions in shared programs: 5773163 -> 5771785 (-0.02%)
> > instructions in affected programs:     21040 -> 19662 (-6.55%)
> > helped:                                717
> >
> > v2: Use the CFG rather than the old instructions list.  Presumably
> >     the placeholder halt will be in the last basic block.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_fs.cpp | 42 ++++++++++++++++++++++++++++++++++++
> >  src/mesa/drivers/dri/i965/brw_fs.h   |  1 +
> >  2 files changed, 43 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > index 9df1650..21e1e82 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> > @@ -2558,6 +2558,47 @@ fs_visitor::opt_register_renaming()
> >     return progress;
> >  }
> >  
> > +/**
> > + * Remove redundant or useless discard jumps.
> > + *
> > + * For example, we can eliminate jumps in the following sequence:
> > + *
> > + * discard-jump       (redundant with the next jump)
> > + * discard-jump       (useless; jumps to the next instruction)
> > + * placeholder-halt
> > + */
> > +bool
> > +fs_visitor::opt_redundant_discard_jumps()
> > +{
> > +   bool progress = false;
> > +
> > +   bblock_t *last_bblock = cfg->blocks[cfg->num_blocks - 1];
> > +
> > +   fs_inst *placeholder_halt = NULL;
> > +   foreach_inst_in_block_reverse(fs_inst, inst, last_bblock) {
> > +      if (inst->opcode == FS_OPCODE_PLACEHOLDER_HALT) {
> > +         placeholder_halt = inst;
> > +         break;
> > +      }
> > +   }
> > +
> > +   if (!placeholder_halt)
> > +      return false;
> > +
> > +   /* Delete any HALTs immediately before the placeholder halt. */
> > +   for (fs_inst *prev = (fs_inst *) placeholder_halt->prev;
> > +        prev->opcode == FS_OPCODE_DISCARD_JUMP;
> > +        prev = (fs_inst *) placeholder_halt->prev) {
> > +      prev->remove(last_bblock);
> > +      progress = true;
> > +   }
> 
> My only question in this series was "what if the placeholder halt is the
> first instruction in the block?"  Shouldn't you be checking for a start
> sentinel?

Yep.  It's certainly not common.  But void main() {} hits that case.

I've changed the loop condition to:
!prev->is_head_sentinel() && prev->opcode == FS_OPCODE_DISCARD_JUMP;

> Other than that, this series is:
> 
> Reviewed-by: Eric Anholt <eric at anholt.net>
> 
> and I look forward to using the NIR intrinsic in my TGSI support.

Thanks!
-------------- 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/20150224/7336529c/attachment.sig>