[Mesa-dev] [PATCH 2/2] nir: Add support for CSE on textures.

[Jason Ekstrand]

On Aug 12, 2015 4:21 PM, "Eric Anholt" <eric at anholt.net> wrote:
>
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
> > On Wed, Aug 12, 2015 at 11:55 AM, Eric Anholt <eric at anholt.net> wrote:
> >> NIR instruction count results on i965:
> >> total instructions in shared programs: 1261954 -> 1261937 (-0.00%)
> >> instructions in affected programs:     455 -> 438 (-3.74%)
> >>
> >> One in yofrankie, two in tropics.  Apparently i965 had also optimized
all
> >> of these out anyway.
> >>
> >> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> >> ---
> >>  src/glsl/nir/nir_opt_cse.c | 43
+++++++++++++++++++++++++++++++++++++++----
> >>  1 file changed, 39 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/glsl/nir/nir_opt_cse.c b/src/glsl/nir/nir_opt_cse.c
> >> index 553906e..864795c 100644
> >> --- a/src/glsl/nir/nir_opt_cse.c
> >> +++ b/src/glsl/nir/nir_opt_cse.c
> >> @@ -86,8 +86,41 @@ nir_instrs_equal(nir_instr *instr1, nir_instr
*instr2)
> >>        }
> >>        return true;
> >>     }
> >> -   case nir_instr_type_tex:
> >> -      return false;
> >> +   case nir_instr_type_tex: {
> >> +      nir_tex_instr *tex1 = nir_instr_as_tex(instr1);
> >> +      nir_tex_instr *tex2 = nir_instr_as_tex(instr2);
> >> +
> >> +      if (tex1->op != tex2->op)
> >> +         return false;
> >> +
> >> +      if (tex1->num_srcs != tex2->num_srcs)
> >> +         return false;
> >> +      for (unsigned i = 0; i < tex1->num_srcs; i++) {
> >> +         if (tex1->src[i].src_type != tex2->src[i].src_type ||
> >> +             !nir_srcs_equal(tex1->src[i].src, tex2->src[i].src)) {
> >> +            return false;
> >
> > We should probably loop over both arrays of sources and match by
> > source type instead of assuming that they are always in the same
> > order.  They *probably* are, but you can pretty easily get cases where
> > they aren't.
> >
> > Otherwise, this looks fine to me.
>
> What circumstance would produce arrays in different order?  I can't
> imagine one, so conservative seems appropriate to me.

I was remembering a place that adds a source but that place is
nir_lower_samplers and its probably the only place that adds the sampler
index source type.  Therefore, it should be justjust as deterministic as
whatever generated it (glsl ir or mesa ir).  I think I'd still rather the
double loop since there is no defined order, but I'm not going to complain
too much.  Either way,

Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>

You can apply that to the patch that zeros texture instructions too.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150812/fa4fecb8/attachment.html>