[Mesa-dev] [PATCH 112/133] nir: Add an algebraic optimization pass

Tue Jan 6 15:25:07 PST 2015

On Mon, Jan 5, 2015 at 11:38 PM, Matt Turner <mattst88 at gmail.com> wrote:

> On Mon, Dec 15, 2014 at 10:12 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > This pass uses the previously built algebraic transformations framework
> and
> > should act as an example for anyone else wanting to make an algebraic
> > transformation pass for NIR.
> > ---
> >  src/glsl/Makefile.am                     | 10 ++++-
> >  src/glsl/Makefile.sources                |  6 ++-
> >  src/glsl/nir/nir.h                       |  2 +
> >  src/glsl/nir/nir_opt_algebraic.py        | 67
> ++++++++++++++++++++++++++++++++
> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |  2 +-
> >  5 files changed, 84 insertions(+), 3 deletions(-)
> >  create mode 100644 src/glsl/nir/nir_opt_algebraic.py
> >
> > diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
> > index 7f62573..bddec9c 100644
> > --- a/src/glsl/Makefile.am
> > +++ b/src/glsl/Makefile.am
> > @@ -25,6 +25,7 @@ AM_CPPFLAGS = \
> >         -I$(top_srcdir)/src/mapi \
> >         -I$(top_srcdir)/src/mesa/ \
> >         -I$(top_srcdir)/src/glsl/glcpp \
> > +       -I$(top_srcdir)/src/glsl/nir \
> >         -I$(top_srcdir)/src/gtest/include \
> >         $(DEFINES)
> >  AM_CFLAGS = $(VISIBILITY_CFLAGS)
> > @@ -205,7 +206,8 @@ BUILT_SOURCES =
>        \
> >         glsl_parser.cpp                                 \
> >         glsl_lexer.cpp                                  \
> >         glcpp/glcpp-parse.c                             \
> > -       glcpp/glcpp-lex.c
> > +       glcpp/glcpp-lex.c                               \
> > +        nir/nir_opt_algebraic.c
>
> Tabs in Makefile.am.
>

Fixed.

>
> >  CLEANFILES =                                           \
> >         glcpp/glcpp-parse.h                             \
> >         glsl_parser.h                                   \
> > @@ -217,3 +219,9 @@ clean-local:
> >  dist-hook:
> >         $(RM) glcpp/tests/*.out
> >         $(RM) glcpp/tests/subtest*/*.out
> > +
> > +nir/nir_opt_algebraic.c: nir/nir_opt_algebraic.py nir/nir_algebraic.py
> > +       $(AM_V_GEN)set -e;
>         \
>
> I don't think you need this?
>

No, probably don't

> > +       $(MKDIR_P) `dirname $@`;
>         \
>
> Just "$(MKDIR_P) nir" I think? I don't think you need the line
> continuations either?
>

Sure.  That works fine

>
> > +       $(PYTHON2) $(PYTHON_FLAGS) $(srcdir)/nir/nir_opt_algebraic.py >
> $@.tmp; \
> > +       mv $@.tmp $@;
>
> Just generate the file directly?
>

Sure.  I've had reasons for doing that in the past, but I don't think it's
needed here.

>
> > diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> > index 201330d..d243f12 100644
> > --- a/src/glsl/Makefile.sources
> > +++ b/src/glsl/Makefile.sources
> > @@ -13,6 +13,9 @@ LIBGLCPP_GENERATED_FILES = \
> >         $(GLSL_BUILDDIR)/glcpp/glcpp-lex.c \
> >         $(GLSL_BUILDDIR)/glcpp/glcpp-parse.c
> >
> > +NIR_GENERATED_FILES = \
> > +       $(GLSL_BUILDDIR)/nir/nir_opt_algebraic.c
> > +
> >  NIR_FILES = \
> >         $(GLSL_SRCDIR)/nir/nir.c \
> >         $(GLSL_SRCDIR)/nir/nir.h \
> > @@ -47,7 +50,8 @@ NIR_FILES = \
> >         $(GLSL_SRCDIR)/nir/nir_to_ssa.c \
> >         $(GLSL_SRCDIR)/nir/nir_validate.c \
> >         $(GLSL_SRCDIR)/nir/nir_types.cpp \
> > -       $(GLSL_SRCDIR)/nir/glsl_to_nir.cpp
> > +       $(GLSL_SRCDIR)/nir/glsl_to_nir.cpp \
> > +        $(NIR_GENERATED_FILES)
>
> Use tabs
>

Fixed.

>
> >
> >  # libglsl
> >
> > diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h
> > index b3c61ca..630e841 100644
> > --- a/src/glsl/nir/nir.h
> > +++ b/src/glsl/nir/nir.h
> > @@ -1402,6 +1402,8 @@ void nir_convert_to_ssa_impl(nir_function_impl
> *impl);
> >  void nir_convert_to_ssa(nir_shader *shader);
> >  void nir_convert_from_ssa(nir_shader *shader);
> >
> > +bool nir_opt_algebraic(nir_shader *shader);
> > +
> >  bool nir_opt_global_to_local(nir_shader *shader);
> >
> >  bool nir_copy_prop_impl(nir_function_impl *impl);
> > diff --git a/src/glsl/nir/nir_opt_algebraic.py
> b/src/glsl/nir/nir_opt_algebraic.py
> > new file mode 100644
> > index 0000000..e829f37
> > --- /dev/null
> > +++ b/src/glsl/nir/nir_opt_algebraic.py
> > @@ -0,0 +1,67 @@
> > +#! /usr/bin/env python
> > +#
> > +# Copyright (C) 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:
> > +#    Jason Ekstrand (jason at jlekstrand.net)
> > +
> > +import nir_algebraic
> > +
> > +# Convenience variables
> > +a = 'a'
> > +b = 'b'
> > +c = 'c'
> > +d = 'd'
> > +
> > +# Written in the form (<search>, <replace>) where <search> is an
> expression
> > +# and <replace> is either an expression or a value.  An expression is
> > +# defined as a tuple of the form (<op>, <src0>, <src1>, <src2>, <src3>)
> > +# where each source is either an expression or a value.  A value can be
> > +# either a numeric constant or a string representing a variable name.
> For
> > +# constants, you have to be careful to make sure that it is the right
> type
> > +# because python is unaware of the source and destination types of the
> > +# opcodes.
> > +
> > +optimizations = [
> > +   (('fadd', a, 0.0), a),
> > +   (('iadd', a, 0), a),
> > +   (('fmul', a, 0.0), 0.0),
> > +   (('imul', a, 0), 0),
> > +   (('fmul', a, 1.0), a),
> > +   (('imul', a, 1), a),
> > +   (('fmul', a, -1.0), ('fneg', a)),
> > +   (('imul', a, -1), ('ineg', a)),
> > +   (('ffma', 0.0, a, b), b),
> > +   (('ffma', a, 0.0, b), b),
> > +   (('ffma', a, b, 0.0), ('fmul', a, b)),
> > +   (('flrp', a, b, 0.0), a),
> > +   (('flrp', a, b, 1.0), b),
> > +   (('flrp', a, a, b), a),
> > +   (('flrp', 0.0, a, b), ('fmul', a, b)),
> > +   (('fadd', ('fmul', a, b), c), ('ffma', a, b, c)),
> > +   (('fge', ('fneg', ('fabs', a)), 0.0), ('feq', a, 0.0)),
> > +   (('fmin', ('fmax', a, 1.0), 0.0), ('fsat', a)),
> > +# This one may not be exact
> > +   (('feq', ('fadd', a, b), 0.0), ('feq', a, ('fneg', b))),
> > +]
> > +
> > +print nir_algebraic.AlgebraicPass("nir_opt_algebraic",
> optimizations).render()
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > index ec1ef28..039dd4d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> > @@ -53,7 +53,7 @@ fs_visitor::emit_nir_code()
> >        nir_validate_shader(nir);
> >        progress |= nir_opt_peephole_select(nir);
> >        nir_validate_shader(nir);
> > -      progress |= nir_opt_peephole_ffma(nir);
> > +      progress |= nir_opt_algebraic(nir);
> >        nir_validate_shader(nir);
> >     } while (progress);
> >
> > --
> > 2.2.0
>
> With those fixed,
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>

Thanks for looking through it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150106/67e2dca7/attachment.html>