On Aug 12, 2015 4:21 PM, "Eric Anholt" <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> wrote: > > Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes: > > > On Wed, Aug 12, 2015 at 11:55 AM, Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> 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 <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> > >> --- > >> 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 <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> You can apply that to the patch that zeros texture instructions too.