[Mesa-dev] [PATCH 4/5] i965/gs: Use new NIR intrinsics.

[Kenneth Graunke]

On Monday, September 07, 2015 11:06:54 AM Jason Ekstrand wrote:
> On Thu, Sep 3, 2015 at 1:48 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> > By performing the vertex counting in NIR, we're able to elide a ton of
> > useless safety checks around every EmitVertex() call:
> >
> > total instructions in shared programs: 3952 -> 3720 (-5.87%)
> > instructions in affected programs:     3491 -> 3259 (-6.65%)
> > helped:                                11
> > HURT:                                  0
> >
> > Improves performance in Gl32GSCloth by 0.671742% +/- 0.142202% (n=621)
> > on Haswell GT3e at 1024x768.
> >
> > This should also make it easier to implement Broadwell's "Static Vertex
> > Count" feature someday.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >  src/mesa/drivers/dri/i965/brw_nir.c               |  5 ++++
> >  src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp     | 13 +++++++++--
> >  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp | 28 ++++++++++++-----------
> >  src/mesa/drivers/dri/i965/gen6_gs_visitor.cpp     | 28 ++++++++++++++---------
> >  4 files changed, 48 insertions(+), 26 deletions(-)
> >
> > There are a couple regressions left with this patch, which are actually bugs
> > in the NIR control flow graph modification code.  I wrote a fix, which IIRC
> > got this to zero regressions.  But I've found even more bugs in the NIR CFG
> > code, so I'm working on yet more fixes.  I'll sort that out before landing
> > this, but figured I may as well send it out for review in the meantime - I've
> > had these patches a long time.
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
> > index 8f3edc5..15e79ba 100644
> > --- a/src/mesa/drivers/dri/i965/brw_nir.c
> > +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> > @@ -93,6 +93,11 @@ brw_create_nir(struct brw_context *brw,
> >     }
> >     nir_validate_shader(nir);
> >
> > +   if (stage == MESA_SHADER_GEOMETRY) {
> > +      nir_lower_gs_intrinsics(nir);
> > +      nir_validate_shader(nir);
> > +   }
> > +
> >     nir_lower_global_vars_to_local(nir);
> >     nir_validate_shader(nir);
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp b/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
> > index 8a8dd57..4f4e1e1 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_nir.cpp
> > @@ -92,16 +92,25 @@ vec4_gs_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
> >     src_reg src;
> >
> >     switch (instr->intrinsic) {
> > -   case nir_intrinsic_emit_vertex: {
> > +   case nir_intrinsic_emit_vertex_with_counter: {
> > +      this->vertex_count =
> > +         retype(get_nir_src(instr->src[0], 1), BRW_REGISTER_TYPE_UD);
> >        int stream_id = instr->const_index[0];
> >        gs_emit_vertex(stream_id);
> >        break;
> >     }
> >
> > -   case nir_intrinsic_end_primitive:
> > +   case nir_intrinsic_end_primitive_with_counter:
> > +      this->vertex_count =
> > +         retype(get_nir_src(instr->src[0], 1), BRW_REGISTER_TYPE_UD);
> 
> It seems kind of odd to me that we are seetting a vertex_count state
> variable in this case and in the one above.  Wouldn't it be better to
> pass the vertex count in to end_primitive() and emit_vertex()?  That
> can be a refactor for another day though; after we delete the old
> vec4_visitor code.

Oh, I wholly agree.  I just did this because this->vertex_count already
existed, and was used in the appropriate places, so it was the simplest
path forward.

I'm totally in favor of deleting this->vertex_count after we delete the
GLSL IR visitor support.

> Modulo getting the CF bugs fixed, this series is
> 
> Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

Thanks Jason!
-------------- 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/20150908/00db4bd7/attachment.sig>