[Mesa-dev] [PATCH 08/11] nir: Add unit tests for control flow graphs.

Connor Abbott cwabbott0 at gmail.com
Tue Sep 22 21:45:42 PDT 2015


On Tue, Sep 22, 2015 at 11:01 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/glsl/Makefile.am                      |  14 +++
>  src/glsl/nir/tests/control_flow_tests.cpp | 155 ++++++++++++++++++++++++++++++
>  2 files changed, 169 insertions(+)
>  create mode 100644 src/glsl/nir/tests/control_flow_tests.cpp
>
> For now, this only adds a single test.  It was broken before this series,
> and now it works.  We certainly need to add more tests; I'm not sure whether
> it makes sense to commit this now or later.
>
> Suggestions for mean things to test are welcome.


Oh boy, I've thought of so many in the past... if you're not inspired,
I'd suggest going through nir_control_flow.c and figuring out how each
of the possible paths can be taken, especially in the lower-level
routines. If it's not relatively obvious or likely to get hit (and I
know for a fact these cases do exist), then we should probably have a
unit test that exercises it. It's too late tonight, but I should
probably do that as well. Also, it might also be a good idea to also
test the bug fixed in patch 03. I don't think adding extra tests
should hold back this getting committed, though -- leaving it around
uncommitted will probably increase the chance that it becomes stale
and make it harder for others (aka me and possibly Jason) to add
tests.

>
> diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
> index 1aa9caa..3265391 100644
> --- a/src/glsl/Makefile.am
> +++ b/src/glsl/Makefile.am
> @@ -50,12 +50,14 @@ EXTRA_DIST = tests glcpp/tests README TODO glcpp/README     \
>         nir/nir_opcodes_c.py                            \
>         nir/nir_opcodes_h.py                            \
>         nir/nir_opt_algebraic.py                        \
> +       nir/tests                                       \
>         SConscript
>
>  include Makefile.sources
>
>  TESTS = glcpp/tests/glcpp-test                         \
>         glcpp/tests/glcpp-test-cr-lf                    \
> +        nir/tests/control_flow_tests                   \
>         tests/blob-test                                 \
>         tests/general-ir-test                           \
>         tests/optimization-test                         \
> @@ -70,6 +72,7 @@ noinst_LTLIBRARIES = libnir.la libglsl.la libglcpp.la
>  check_PROGRAMS =                                       \
>         glcpp/glcpp                                     \
>         glsl_test                                       \
> +       nir/tests/control_flow_tests                    \
>         tests/blob-test                                 \
>         tests/general-ir-test                           \
>         tests/sampler-types-test                        \
> @@ -263,3 +266,14 @@ nir/nir_opcodes.c: nir/nir_opcodes.py nir/nir_opcodes_c.py
>  nir/nir_opt_algebraic.c: nir/nir_opt_algebraic.py nir/nir_algebraic.py
>         $(MKDIR_GEN)
>         $(PYTHON_GEN) $(srcdir)/nir/nir_opt_algebraic.py > $@
> +
> +nir_tests_control_flow_tests_SOURCES =                 \
> +       nir/tests/control_flow_tests.cpp
> +nir_tests_control_flow_tests_CFLAGS =                  \
> +       $(PTHREAD_CFLAGS)
> +nir_tests_control_flow_tests_LDADD =                   \
> +       $(top_builddir)/src/gtest/libgtest.la           \
> +       $(top_builddir)/src/glsl/libnir.la              \
> +       $(top_builddir)/src/libglsl_util.la             \
> +       $(top_builddir)/src/util/libmesautil.la         \
> +       $(PTHREAD_LIBS)
> diff --git a/src/glsl/nir/tests/control_flow_tests.cpp b/src/glsl/nir/tests/control_flow_tests.cpp
> new file mode 100644
> index 0000000..b9f90e6
> --- /dev/null
> +++ b/src/glsl/nir/tests/control_flow_tests.cpp
> @@ -0,0 +1,155 @@
> +/*
> + * Copyright © 2015 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.
> + */
> +#include <gtest/gtest.h>
> +#include "nir.h"
> +#include "nir_builder.h"
> +
> +class nir_cf_test : public ::testing::Test {
> +protected:
> +   nir_cf_test();
> +   ~nir_cf_test();
> +
> +   nir_builder b;
> +   nir_shader *shader;
> +   nir_function_impl *impl;
> +};
> +
> +nir_cf_test::nir_cf_test()
> +{
> +   static const nir_shader_compiler_options options = { };
> +   shader = nir_shader_create(NULL, MESA_SHADER_VERTEX, &options);
> +   nir_function *func = nir_function_create(shader, "main");
> +   nir_function_overload *overload = nir_function_overload_create(func);
> +   impl = nir_function_impl_create(overload);
> +
> +   nir_builder_init(&b, impl);
> +}
> +
> +nir_cf_test::~nir_cf_test()
> +{
> +   ralloc_free(shader);
> +}
> +
> +TEST_F(nir_cf_test, delete_break_in_loop)
> +{
> +   /* Create IR:
> +    *
> +    * while (...) { break; }
> +    */
> +   nir_loop *loop = nir_loop_create(shader);
> +   nir_cf_node_insert(nir_after_cf_list(&impl->body), &loop->cf_node);
> +
> +   b.cursor = nir_after_cf_list(&loop->body);
> +
> +   nir_jump_instr *jump = nir_jump_instr_create(shader, nir_jump_break);
> +   nir_builder_instr_insert(&b, &jump->instr);
> +
> +   /* At this point, we should have:
> +    *
> +    * impl main {
> +    *         block block_0:
> +    *         // preds:
> +    *         // succs: block_1
> +    *         loop {
> +    *                 block block_1:
> +    *                 // preds: block_0
> +    *                 break
> +    *                 // succs: block_2
> +    *         }
> +    *         block block_2:
> +    *         // preds: block_1
> +    *         // succs: block_3
> +    *         block block_3:
> +    * }
> +    */
> +   nir_block *block_0 = nir_start_block(impl);
> +   nir_block *block_1 = nir_cf_node_as_block(nir_loop_first_cf_node(loop));
> +   nir_block *block_2 = nir_cf_node_as_block(nir_cf_node_next(&loop->cf_node));
> +   nir_block *block_3 = impl->end_block;
> +   ASSERT_EQ(nir_cf_node_block, block_0->cf_node.type);
> +   ASSERT_EQ(nir_cf_node_block, block_1->cf_node.type);
> +   ASSERT_EQ(nir_cf_node_block, block_2->cf_node.type);
> +   ASSERT_EQ(nir_cf_node_block, block_3->cf_node.type);
> +
> +   /* Verify the successors and predecessors. */
> +   EXPECT_EQ(block_1, block_0->successors[0]);
> +   EXPECT_EQ(NULL,    block_0->successors[1]);
> +   EXPECT_EQ(block_2, block_1->successors[0]);
> +   EXPECT_EQ(NULL,    block_1->successors[1]);
> +   EXPECT_EQ(block_3, block_2->successors[0]);
> +   EXPECT_EQ(NULL,    block_2->successors[1]);
> +   EXPECT_EQ(NULL,    block_3->successors[0]);
> +   EXPECT_EQ(NULL,    block_3->successors[1]);
> +   EXPECT_EQ(0,       block_0->predecessors->entries);
> +   EXPECT_EQ(1,       block_1->predecessors->entries);
> +   EXPECT_EQ(1,       block_2->predecessors->entries);
> +   EXPECT_EQ(1,       block_3->predecessors->entries);
> +   EXPECT_TRUE(_mesa_set_search(block_1->predecessors, block_0));
> +   EXPECT_TRUE(_mesa_set_search(block_2->predecessors, block_1));
> +   EXPECT_TRUE(_mesa_set_search(block_3->predecessors, block_2));
> +
> +   nir_print_shader(shader, stderr);
> +
> +   /* Now remove the break. */
> +   nir_instr_remove(&jump->instr);
> +
> +   nir_print_shader(shader, stderr);
> +
> +   /* At this point, we should have:
> +    *
> +    * impl main {
> +    *         block block_0:
> +    *         // preds:
> +    *         // succs: block_1
> +    *         loop {
> +    *                 block block_1:
> +    *                 // preds: block_0 block_1
> +    *                 // succs: block_1
> +    *         }
> +    *         block block_2:
> +    *         // preds: block_1
> +    *         // succs: block_3
> +    *         block block_3:
> +    * }
> +    *
> +    * Re-verify the predecessors and successors.
> +    */
> +   EXPECT_EQ(block_1, block_0->successors[0]);
> +   EXPECT_EQ(NULL,    block_0->successors[1]);
> +   EXPECT_EQ(block_1, block_1->successors[0]); /* back to itself */
> +   EXPECT_EQ(block_2, block_1->successors[1]); /* fake successor */
> +   EXPECT_EQ(block_3, block_2->successors[0]);
> +   EXPECT_EQ(NULL,    block_2->successors[1]);
> +   EXPECT_EQ(NULL,    block_3->successors[0]);
> +   EXPECT_EQ(NULL,    block_3->successors[1]);
> +   EXPECT_EQ(0,       block_0->predecessors->entries);
> +   EXPECT_EQ(2,       block_1->predecessors->entries);
> +   EXPECT_EQ(1,       block_2->predecessors->entries);
> +   EXPECT_EQ(1,       block_3->predecessors->entries);
> +   EXPECT_TRUE(_mesa_set_search(block_1->predecessors, block_0));
> +   EXPECT_TRUE(_mesa_set_search(block_1->predecessors, block_1));
> +   EXPECT_TRUE(_mesa_set_search(block_2->predecessors, block_1));
> +   EXPECT_TRUE(_mesa_set_search(block_3->predecessors, block_2));
> +
> +   nir_metadata_require(impl, nir_metadata_dominance);
> +}
> --
> 2.5.3
>


More information about the mesa-dev mailing list