[Mesa-dev] [PATCH 26/26] nir/dead_variables: Configurably work with any variable mode

Rob Clark robdclark at gmail.com
Sat Mar 26 22:00:31 UTC 2016


On Sat, Mar 26, 2016 at 5:43 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>
> On Sat, Mar 26, 2016 at 8:22 AM, Rob Clark <robdclark at gmail.com> wrote:
>>
>> On Fri, Mar 25, 2016 at 7:12 PM, Jason Ekstrand <jason at jlekstrand.net>
>> 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);
>>
>> 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)..
>
>
> 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.
>
>>
>> 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..
>
>
> 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.

I dug it up, and the issue is in nir_variable_data:

      nir_variable_mode mode:5;

ofc we could just give more bits to that and be done...

BR,
-R

> --Jason
>
>>
>> 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
>> > mesa-dev at lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>


More information about the mesa-dev mailing list