<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Sat, Mar 26, 2016 at 8:22 AM, Rob Clark <<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Mar 25, 2016 at 7:12 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > The old version of the pass only worked on globals and locals and always > left inputs, outputs, uniforms, etc. alone. > --- > src/compiler/nir/nir.h | 2 +- > src/compiler/nir/nir_remove_dead_variables.c | 33 ++++++++++++++++++++-------- > src/gallium/drivers/freedreno/ir3/ir3_nir.c | 2 +- > src/gallium/drivers/vc4/vc4_program.c | 2 +- > src/mesa/drivers/dri/i965/brw_nir.c | 2 +- > 5 files changed, 28 insertions(+), 13 deletions(-) > > diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h > index 03c1f76..d9e0d67 100644 > --- a/src/compiler/nir/nir.h > +++ b/src/compiler/nir/nir.h > @@ -2257,7 +2257,7 @@ nir_src *nir_get_io_vertex_index_src(nir_intrinsic_instr *instr); > > void nir_lower_vars_to_ssa(nir_shader *shader); > > -bool nir_remove_dead_variables(nir_shader *shader); > +bool nir_remove_dead_variables(nir_shader *shader, nir_variable_mode mode); > > void nir_move_vec_src_uses_to_dest(nir_shader *shader); > bool nir_lower_vec_to_movs(nir_shader *shader); > diff --git a/src/compiler/nir/nir_remove_dead_variables.c b/src/compiler/nir/nir_remove_dead_variables.c > index 6519268..ad69de8 100644 > --- a/src/compiler/nir/nir_remove_dead_variables.c > +++ b/src/compiler/nir/nir_remove_dead_variables.c > @@ -120,7 +120,7 @@ remove_dead_vars(struct exec_list *var_list, struct set *live) > } > > bool > -nir_remove_dead_variables(nir_shader *shader) > +nir_remove_dead_variables(nir_shader *shader, nir_variable_mode mode) > { > bool progress = false; > struct set *live = > @@ -128,15 +128,30 @@ nir_remove_dead_variables(nir_shader *shader) > > add_var_use_shader(shader, live); > > - progress = remove_dead_vars(&shader->globals, live) || progress; > + if (mode == nir_var_uniform || mode == nir_var_all) > + progress = remove_dead_vars(&shader->uniforms, live) || progress; > > - nir_foreach_function(shader, function) { > - if (function->impl) { > - if (remove_dead_vars(&function->impl->locals, live)) { > - nir_metadata_preserve(function->impl, nir_metadata_block_index | > - nir_metadata_dominance | > - nir_metadata_live_ssa_defs); > - progress = true; > + if (mode == nir_var_shader_in || mode == nir_var_all) > + progress = remove_dead_vars(&shader->inputs, live) || progress; > + > + if (mode == nir_var_shader_out || mode == nir_var_all) > + progress = remove_dead_vars(&shader->outputs, live) || progress; > + > + if (mode == nir_var_global || mode == nir_var_all) > + progress = remove_dead_vars(&shader->globals, live) || progress; > + > + if (mode == nir_var_system_value || mode == nir_var_all) > + progress = remove_dead_vars(&shader->system_values, live) || progress; > + > + if (mode == nir_var_local || mode == nir_var_all) { > + nir_foreach_function(shader, function) { > + if (function->impl) { > + if (remove_dead_vars(&function->impl->locals, live)) { > + nir_metadata_preserve(function->impl, nir_metadata_block_index | > + nir_metadata_dominance | > + nir_metadata_live_ssa_defs); > + progress = true; > + } > } > } > } > diff --git a/src/gallium/drivers/freedreno/ir3/ir3_nir.c b/src/gallium/drivers/freedreno/ir3/ir3_nir.c > index 73c65d6..04c42ff 100644 > --- a/src/gallium/drivers/freedreno/ir3/ir3_nir.c > +++ b/src/gallium/drivers/freedreno/ir3/ir3_nir.c > @@ -141,7 +141,7 @@ ir3_optimize_nir(struct ir3_shader *shader, nir_shader *s, > > } while (progress); > > - OPT_V(s, nir_remove_dead_variables); > + OPT_V(s, nir_remove_dead_variables, nir_var_local); </div></div>I guess need to add nir_var_global too? Or maybe just nir_var_all for good measure.. ttn doesn't generate locals but does globals (and w/ gtn at least I'll start seeing both).. </blockquote><div> </div><div>No, you don't. Assuming a single funciton, the call to globals_to_locals above will take care of that for you. Same for i965 and vc4. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> btw, I do remember for lower_io I wanted the mode to be a bitmask so we could more easily deal w/ multiple modes in the same pass, but that exceeded # of bits somewhere or other. I suppose maybe nir_var_all is good enough.. offhand I can't think of a scenario where we want to lower some but not all.. </blockquote><div> </div><div>I'd rather do that too. I don't remember why we didn't before (too lazy to search the mailing list). It shouldn't overflow a uint32_t, there are only 9 modes last I checked. We may add more, but it'll be a while before we add 22 more. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> BR, -R > > if (fd_mesa_debug & FD_DBG_DISASM) { > debug_printf("----------------------\n"); > diff --git a/src/gallium/drivers/vc4/vc4_program.c b/src/gallium/drivers/vc4/vc4_program.c > index 71a1ebbb..6418bc5 100644 > --- a/src/gallium/drivers/vc4/vc4_program.c > +++ b/src/gallium/drivers/vc4/vc4_program.c > @@ -1881,7 +1881,7 @@ vc4_shader_ntq(struct vc4_context *vc4, enum qstage stage, > > vc4_optimize_nir(c->s); > > - nir_remove_dead_variables(c->s); > + nir_remove_dead_variables(c->s, nir_var_local); > > nir_convert_from_ssa(c->s, true); > > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c > index c62840a..8392189 100644 > --- a/src/mesa/drivers/dri/i965/brw_nir.c > +++ b/src/mesa/drivers/dri/i965/brw_nir.c > @@ -468,7 +468,7 @@ brw_preprocess_nir(nir_shader *nir, bool is_scalar) > /* Get rid of split copies */ > nir = nir_optimize(nir, is_scalar); > > - OPT(nir_remove_dead_variables); > + OPT(nir_remove_dead_variables, nir_var_local); > > return nir; > } > -- > 2.5.0.400.gff86faf > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>