On Tue, Sep 22, 2015 at 11:01 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> Signed-off-by: Kenneth Graunke <kenn...@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
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
