On Wed, Jan 4, 2017 at 3:20 PM, Timothy Arceri <timothy.arc...@collabora.com
> wrote:
> On Mon, 2016-12-12 at 19:39 -0800, Jason Ekstrand wrote:
> > ---
> > src/compiler/nir/nir_remove_dead_variables.c | 66
> > ++++++++++++++++++++++++----
> > 1 file changed, 58 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/compiler/nir/nir_remove_dead_variables.c
> > b/src/compiler/nir/nir_remove_dead_variables.c
> > index f7429eb..d22b7f5 100644
> > --- a/src/compiler/nir/nir_remove_dead_variables.c
> > +++ b/src/compiler/nir/nir_remove_dead_variables.c
> > @@ -31,9 +31,27 @@ static void
> > add_var_use_intrinsic(nir_intrinsic_instr *instr, struct set *live)
> > {
> > unsigned num_vars = nir_intrinsic_infos[instr-
> > >intrinsic].num_variables;
> > - for (unsigned i = 0; i < num_vars; i++) {
> > - nir_variable *var = instr->variables[i]->var;
> > - _mesa_set_add(live, var);
> > +
> > + switch (instr->intrinsic) {
> > + case nir_intrinsic_copy_var:
> > + _mesa_set_add(live, instr->variables[1]->var);
> > + /* Fall through */
> > + case nir_intrinsic_store_var: {
> > + /* The first source in both copy_var and store_var is the
> > destination.
> > + * If the variable is a local that never escapes the shader,
> > then we
> > + * don't mark it as live for just a store.
> > + */
> > + nir_variable_mode mode = instr->variables[0]->var->data.mode;
> > + if (!(mode & (nir_var_local | nir_var_global |
> > nir_var_shared)))
>
> So you have nir_var_shared here but I think you are missing the bit to
> add:
>
> if (modes & nir_var_shared)
> progress = remove_dead_vars(&shader->shared, live) || progress;
>
That needs to be its own patch, but sure.
>
> Otherwise won't the var remain while the write instruction is removed?
>
>
> > + _mesa_set_add(live, instr->variables[0]->var);
> > + break;
> > + }
> > +
> > + default:
> > + for (unsigned i = 0; i < num_vars; i++) {
> > + _mesa_set_add(live, instr->variables[i]->var);
> > + }
> > + break;
> > }
> > }
> >
> > @@ -94,6 +112,31 @@ add_var_use_shader(nir_shader *shader, struct set
> > *live)
> > }
> > }
> >
> > +static void
> > +remove_dead_var_writes(nir_shader *shader, struct set *live)
> > +{
> > + nir_foreach_function(function, shader) {
> > + if (!function->impl)
> > + continue;
> > +
> > + nir_foreach_block(block, function->impl) {
> > + nir_foreach_instr_safe(instr, block) {
> > + if (instr->type != nir_instr_type_intrinsic)
> > + continue;
> > +
> > + nir_intrinsic_instr *intrin =
> > nir_instr_as_intrinsic(instr);
> > + if (intrin->intrinsic != nir_intrinsic_copy_var &&
> > + intrin->intrinsic != nir_intrinsic_store_var)
> > + continue;
> > +
> > + /* Stores to dead variables need to be removed */
> > + if (!_mesa_set_search(live, intrin->variables[0]->var))
> > + nir_instr_remove(instr);
> > + }
> > + }
> > + }
> > +}
> > +
> > static bool
> > remove_dead_vars(struct exec_list *var_list, struct set *live)
> > {
> > @@ -138,12 +181,19 @@ nir_remove_dead_variables(nir_shader *shader,
> > nir_variable_mode modes)
> > if (modes & nir_var_local) {
> > nir_foreach_function(function, shader) {
> > if (function->impl) {
> > - if (remove_dead_vars(&function->impl->locals, live)) {
> > - nir_metadata_preserve(function->impl,
> > nir_metadata_block_index |
> > - nir_metadata_do
> > minance |
> > - nir_metadata_li
> > ve_ssa_defs);
> > + if (remove_dead_vars(&function->impl->locals, live))
> > progress = true;
> > - }
> > + }
> > + }
> > + }
> > +
> > + if (progress) {
> > + remove_dead_var_writes(shader, live);
>
> The vars are conditional on modes passed to nir_remove_dead_variables()
> dont we need this here too? Otherwise we will remove the write
> instruction but not the var.
>
Hrm... Yes. I'm thinking that we probably want to set the variable mode to
0 when we delete it and use that to decide when to remove a write. That
way we never run into this problem in the future.
> > +
> > + nir_foreach_function(function, shader) {
> > + if (function->impl) {
> > + nir_metadata_preserve(function->impl,
> > nir_metadata_block_index |
> > + nir_metadata_domin
> > ance);
> > }
> > }
> > }
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
