[Mesa-dev] [PATCH 018/133] nir: add an SSA-based copy propagation pass

[Jason Ekstrand]

On Wed, Dec 17, 2014 at 2:26 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>
> On Wed, Dec 17, 2014 at 5:20 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> >
> > On Wed, Dec 17, 2014 at 1:10 PM, Jason Ekstrand <jason at jlekstrand.net>
> > wrote:
> >>
> >>
> >>
> >> On Wed, Dec 17, 2014 at 12:07 PM, Connor Abbott <cwabbott0 at gmail.com>
> >> wrote:
> >>>
> >>> On Tue, Dec 16, 2014 at 1:04 AM, Jason Ekstrand <jason at jlekstrand.net>
> >>> wrote:
> >>> > From: Connor Abbott <connor.abbott at intel.com>
> >>> >
> >>> > ---
> >>> >  src/glsl/Makefile.sources             |   1 +
> >>> >  src/glsl/nir/nir.h                    |   3 +
> >>> >  src/glsl/nir/nir_opt_copy_propagate.c | 313
> >>> > ++++++++++++++++++++++++++++++++++
> >>> >  3 files changed, 317 insertions(+)
> >>> >  create mode 100644 src/glsl/nir/nir_opt_copy_propagate.c
> >>> >
> >>> > diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> >>> > index 0aaea58..556648b 100644
> >>> > --- a/src/glsl/Makefile.sources
> >>> > +++ b/src/glsl/Makefile.sources
> >>> > @@ -25,6 +25,7 @@ NIR_FILES = \
> >>> >         $(GLSL_SRCDIR)/nir/nir_lower_variables_scalar.c \
> >>> >         $(GLSL_SRCDIR)/nir/nir_opcodes.c \
> >>> >         $(GLSL_SRCDIR)/nir/nir_opcodes.h \
> >>> > +       $(GLSL_SRCDIR)/nir/nir_opt_copy_propagate.c \
> >>> >         $(GLSL_SRCDIR)/nir/nir_opt_global_to_local.c \
> >>> >         $(GLSL_SRCDIR)/nir/nir_print.c \
> >>> >         $(GLSL_SRCDIR)/nir/nir_remove_dead_variables.c \
> >>> > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> >>> > index c2b4724..a5cb5ed 100644
> >>> > --- a/src/glsl/nir/nir.h
> >>> > +++ b/src/glsl/nir/nir.h
> >>> > @@ -1293,6 +1293,9 @@ void nir_convert_to_ssa(nir_shader *shader);
> >>> >
> >>> >  bool nir_opt_global_to_local(nir_shader *shader);
> >>> >
> >>> > +bool nir_copy_prop_impl(nir_function_impl *impl);
> >>> > +bool nir_copy_prop(nir_shader *shader);
> >>> > +
> >>> >  #ifdef __cplusplus
> >>> >  } /* extern "C" */
> >>> >  #endif
> >>> > diff --git a/src/glsl/nir/nir_opt_copy_propagate.c
> >>> > b/src/glsl/nir/nir_opt_copy_propagate.c
> >>> > new file mode 100644
> >>> > index 0000000..a2be047
> >>> > --- /dev/null
> >>> > +++ b/src/glsl/nir/nir_opt_copy_propagate.c
> >>> > @@ -0,0 +1,313 @@
> >>> > +/*
> >>> > + * Copyright © 2014 Intel Corporation
> >>> > + *
> >>> > + * Permission is hereby granted, free of charge, to any person
> >>> > obtaining a
> >>> > + * copy of this software and associated documentation files (the
> >>> > "Software"),
> >>> > + * to deal in the Software without restriction, including without
> >>> > limitation
> >>> > + * the rights to use, copy, modify, merge, publish, distribute,
> >>> > sublicense,
> >>> > + * and/or sell copies of the Software, and to permit persons to whom
> >>> > the
> >>> > + * Software is furnished to do so, subject to the following
> >>> > conditions:
> >>> > + *
> >>> > + * The above copyright notice and this permission notice (including
> >>> > the next
> >>> > + * paragraph) shall be included in all copies or substantial
> portions
> >>> > of the
> >>> > + * Software.
> >>> > + *
> >>> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >>> > EXPRESS OR
> >>> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >>> > MERCHANTABILITY,
> >>> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT
> >>> > SHALL
> >>> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> >>> > OR OTHER
> >>> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >>> > ARISING
> >>> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER
> >>> > DEALINGS
> >>> > + * IN THE SOFTWARE.
> >>> > + *
> >>> > + * Authors:
> >>> > + *    Connor Abbott (cwabbott0 at gmail.com)
> >>> > + *
> >>> > + */
> >>> > +
> >>> > +#include "nir.h"
> >>> > +#include <main/imports.h>
> >>> > +
> >>> > +/**
> >>> > + * SSA-based copy propagation
> >>> > + */
> >>> > +
> >>> > +static bool is_move(nir_alu_instr *instr)
> >>> > +{
> >>> > +   if (instr->op != nir_op_fmov &&
> >>> > +       instr->op != nir_op_imov)
> >>> > +      return false;
> >>> > +
> >>> > +   if (instr->dest.saturate)
> >>> > +      return false;
> >>> > +
> >>> > +   /* we handle modifiers in a separate pass */
> >>>
> >>> This comment is stale now, since this pass should never see
> >>> modifiers... maybe we should replace those if's with asserts to make
> >>> that clear.
> >>
> >>
> >> Easy enough to do.
> >
> >
> > Wait, I just remembered that I left them in intentionally.  We may want
> to
> > run copy prop after lowering to source mods.  In fact, when I wrote
> > lower_to_source_mods, I was lazy and just changed sat instructions to mov
> > and trusted in copy prop to clan up the swizzles for me.  If anything, we
> > may want to make copy prop simply handle them correctly.
> > --Jason
>
> Ok, that's fine. In that case, we should probably have a separate pass
> to move sat/abs/neg out of moves like the comment mentions, similar to
> what we do in i965 fs. Or did you already write one?
>

I don't think that we actually have that problem. Moving them into their
own instructions, copy prop, and lowering back to modifiers should clean
that up just fine.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141217/b98c0dec/attachment.html>