<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 5, 2015 at 11:38 PM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Dec 15, 2014 at 10:12 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> This pass uses the previously built algebraic transformations framework and<br>
> should act as an example for anyone else wanting to make an algebraic<br>
> transformation pass for NIR.<br>
> ---<br>
> src/glsl/Makefile.am | 10 ++++-<br>
> src/glsl/Makefile.sources | 6 ++-<br>
> src/glsl/nir/nir.h | 2 +<br>
> src/glsl/nir/nir_opt_algebraic.py | 67 ++++++++++++++++++++++++++++++++<br>
> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 2 +-<br>
> 5 files changed, 84 insertions(+), 3 deletions(-)<br>
> create mode 100644 src/glsl/nir/nir_opt_algebraic.py<br>
><br>
> diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am<br>
> index 7f62573..bddec9c 100644<br>
> --- a/src/glsl/Makefile.am<br>
> +++ b/src/glsl/Makefile.am<br>
> @@ -25,6 +25,7 @@ AM_CPPFLAGS = \<br>
> -I$(top_srcdir)/src/mapi \<br>
> -I$(top_srcdir)/src/mesa/ \<br>
> -I$(top_srcdir)/src/glsl/glcpp \<br>
> + -I$(top_srcdir)/src/glsl/nir \<br>
> -I$(top_srcdir)/src/gtest/include \<br>
> $(DEFINES)<br>
> AM_CFLAGS = $(VISIBILITY_CFLAGS)<br>
> @@ -205,7 +206,8 @@ BUILT_SOURCES = \<br>
> glsl_parser.cpp \<br>
> glsl_lexer.cpp \<br>
> glcpp/glcpp-parse.c \<br>
> - glcpp/glcpp-lex.c<br>
> + glcpp/glcpp-lex.c \<br>
> + nir/nir_opt_algebraic.c<br>
<br>
</div></div>Tabs in Makefile.am.<br></blockquote><div><br></div><div>Fixed.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> CLEANFILES = \<br>
> glcpp/glcpp-parse.h \<br>
> glsl_parser.h \<br>
> @@ -217,3 +219,9 @@ clean-local:<br>
> dist-hook:<br>
> $(RM) glcpp/tests/*.out<br>
> $(RM) glcpp/tests/subtest*/*.out<br>
> +<br>
> +nir/nir_opt_algebraic.c: nir/nir_opt_algebraic.py nir/nir_algebraic.py<br>
> + $(AM_V_GEN)set -e; \<br>
<br>
</span>I don't think you need this?<br></blockquote><div><br></div><div>No, probably don't<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> + $(MKDIR_P) `dirname $@`; \<br>
<br>
Just "$(MKDIR_P) nir" I think? I don't think you need the line<br>
continuations either?<br></blockquote><div><br></div><div>Sure. That works fine<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> + $(PYTHON2) $(PYTHON_FLAGS) $(srcdir)/nir/nir_opt_algebraic.py > $@.tmp; \<br>
> + mv $@.tmp $@;<br>
<br>
</span>Just generate the file directly?<br></blockquote><div><br></div><div>Sure. I've had reasons for doing that in the past, but I don't think it's needed here.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources<br>
> index 201330d..d243f12 100644<br>
> --- a/src/glsl/Makefile.sources<br>
> +++ b/src/glsl/Makefile.sources<br>
> @@ -13,6 +13,9 @@ LIBGLCPP_GENERATED_FILES = \<br>
> $(GLSL_BUILDDIR)/glcpp/glcpp-lex.c \<br>
> $(GLSL_BUILDDIR)/glcpp/glcpp-parse.c<br>
><br>
> +NIR_GENERATED_FILES = \<br>
> + $(GLSL_BUILDDIR)/nir/nir_opt_algebraic.c<br>
> +<br>
> NIR_FILES = \<br>
> $(GLSL_SRCDIR)/nir/nir.c \<br>
> $(GLSL_SRCDIR)/nir/nir.h \<br>
> @@ -47,7 +50,8 @@ NIR_FILES = \<br>
> $(GLSL_SRCDIR)/nir/nir_to_ssa.c \<br>
> $(GLSL_SRCDIR)/nir/nir_validate.c \<br>
> $(GLSL_SRCDIR)/nir/nir_types.cpp \<br>
> - $(GLSL_SRCDIR)/nir/glsl_to_nir.cpp<br>
> + $(GLSL_SRCDIR)/nir/glsl_to_nir.cpp \<br>
> + $(NIR_GENERATED_FILES)<br>
<br>
</span>Use tabs<br></blockquote><div><br></div><div>Fixed.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div class="h5"><br>
><br>
> # libglsl<br>
><br>
> diff --git a/src/glsl/nir/nir.h b/src/glsl/nir/nir.h<br>
> index b3c61ca..630e841 100644<br>
> --- a/src/glsl/nir/nir.h<br>
> +++ b/src/glsl/nir/nir.h<br>
> @@ -1402,6 +1402,8 @@ void nir_convert_to_ssa_impl(nir_function_impl *impl);<br>
> void nir_convert_to_ssa(nir_shader *shader);<br>
> void nir_convert_from_ssa(nir_shader *shader);<br>
><br>
> +bool nir_opt_algebraic(nir_shader *shader);<br>
> +<br>
> bool nir_opt_global_to_local(nir_shader *shader);<br>
><br>
> bool nir_copy_prop_impl(nir_function_impl *impl);<br>
> diff --git a/src/glsl/nir/nir_opt_algebraic.py b/src/glsl/nir/nir_opt_algebraic.py<br>
> new file mode 100644<br>
> index 0000000..e829f37<br>
> --- /dev/null<br>
> +++ b/src/glsl/nir/nir_opt_algebraic.py<br>
> @@ -0,0 +1,67 @@<br>
> +#! /usr/bin/env python<br>
> +#<br>
> +# Copyright (C) 2014 Intel Corporation<br>
> +#<br>
> +# Permission is hereby granted, free of charge, to any person obtaining a<br>
> +# copy of this software and associated documentation files (the "Software"),<br>
> +# to deal in the Software without restriction, including without limitation<br>
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,<br>
> +# and/or sell copies of the Software, and to permit persons to whom the<br>
> +# Software is furnished to do so, subject to the following conditions:<br>
> +#<br>
> +# The above copyright notice and this permission notice (including the next<br>
> +# paragraph) shall be included in all copies or substantial portions of the<br>
> +# Software.<br>
> +#<br>
> +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR<br>
> +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,<br>
> +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL<br>
> +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER<br>
> +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING<br>
> +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS<br>
> +# IN THE SOFTWARE.<br>
> +#<br>
> +# Authors:<br>
> +# Jason Ekstrand (<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>)<br>
> +<br>
> +import nir_algebraic<br>
> +<br>
> +# Convenience variables<br>
> +a = 'a'<br>
> +b = 'b'<br>
> +c = 'c'<br>
> +d = 'd'<br>
> +<br>
> +# Written in the form (<search>, <replace>) where <search> is an expression<br>
> +# and <replace> is either an expression or a value. An expression is<br>
> +# defined as a tuple of the form (<op>, <src0>, <src1>, <src2>, <src3>)<br>
> +# where each source is either an expression or a value. A value can be<br>
> +# either a numeric constant or a string representing a variable name. For<br>
> +# constants, you have to be careful to make sure that it is the right type<br>
> +# because python is unaware of the source and destination types of the<br>
> +# opcodes.<br>
> +<br>
> +optimizations = [<br>
> + (('fadd', a, 0.0), a),<br>
> + (('iadd', a, 0), a),<br>
> + (('fmul', a, 0.0), 0.0),<br>
> + (('imul', a, 0), 0),<br>
> + (('fmul', a, 1.0), a),<br>
> + (('imul', a, 1), a),<br>
> + (('fmul', a, -1.0), ('fneg', a)),<br>
> + (('imul', a, -1), ('ineg', a)),<br>
> + (('ffma', 0.0, a, b), b),<br>
> + (('ffma', a, 0.0, b), b),<br>
> + (('ffma', a, b, 0.0), ('fmul', a, b)),<br>
> + (('flrp', a, b, 0.0), a),<br>
> + (('flrp', a, b, 1.0), b),<br>
> + (('flrp', a, a, b), a),<br>
> + (('flrp', 0.0, a, b), ('fmul', a, b)),<br>
> + (('fadd', ('fmul', a, b), c), ('ffma', a, b, c)),<br>
> + (('fge', ('fneg', ('fabs', a)), 0.0), ('feq', a, 0.0)),<br>
> + (('fmin', ('fmax', a, 1.0), 0.0), ('fsat', a)),<br>
> +# This one may not be exact<br>
> + (('feq', ('fadd', a, b), 0.0), ('feq', a, ('fneg', b))),<br>
> +]<br>
> +<br>
> +print nir_algebraic.AlgebraicPass("nir_opt_algebraic", optimizations).render()<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> index ec1ef28..039dd4d 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> @@ -53,7 +53,7 @@ fs_visitor::emit_nir_code()<br>
> nir_validate_shader(nir);<br>
> progress |= nir_opt_peephole_select(nir);<br>
> nir_validate_shader(nir);<br>
> - progress |= nir_opt_peephole_ffma(nir);<br>
> + progress |= nir_opt_algebraic(nir);<br>
> nir_validate_shader(nir);<br>
> } while (progress);<br>
><br>
> --<br>
> 2.2.0<br>
<br>
</div></div>With those fixed,<br>
<br>
Reviewed-by: Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>><br></blockquote><div><br></div><div>Thanks for looking through it. <br></div></div><br></div></div>