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

[Connor Abbott]

On Wed, Dec 17, 2014 at 6:02 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>
> 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

I'll trust you since I haven't read the lower_to_source_mods pass in
question, but if that's the case then we should drop this comment or
replace it.