ping On 10/26/2016 07:17 PM, Ian Romanick wrote: > From: Ian Romanick <[email protected]> > > This just makes the output of the standalone compiler a little more > compact. > > v2: Fix indexing typo noticed by Iago. Move the add_neg_to_sub_visitor > to it's own header file. Add a unit test that exercises the visitor. > Both the neg_a_plus_b and neg_a_plus_neg_b tests reproduced the bug that > Iago discovered. > > Signed-off-by: Ian Romanick <[email protected]> > --- > src/compiler/Makefile.glsl.am | 1 + > src/compiler/glsl/opt_add_neg_to_sub.h | 61 ++++++ > src/compiler/glsl/standalone.cpp | 4 + > .../glsl/tests/opt_add_neg_to_sub_test.cpp | 210 > +++++++++++++++++++++ > 4 files changed, 276 insertions(+) > create mode 100644 src/compiler/glsl/opt_add_neg_to_sub.h > create mode 100644 src/compiler/glsl/tests/opt_add_neg_to_sub_test.cpp > > diff --git a/src/compiler/Makefile.glsl.am b/src/compiler/Makefile.glsl.am > index 80dfb73..4de51e4 100644 > --- a/src/compiler/Makefile.glsl.am > +++ b/src/compiler/Makefile.glsl.am > @@ -69,6 +69,7 @@ glsl_tests_general_ir_test_SOURCES = > \ > glsl/tests/builtin_variable_test.cpp \ > glsl/tests/invalidate_locations_test.cpp \ > glsl/tests/general_ir_test.cpp \ > + glsl/tests/opt_add_neg_to_sub_test.cpp \ > glsl/tests/varyings_test.cpp > glsl_tests_general_ir_test_CFLAGS = \ > $(PTHREAD_CFLAGS) > diff --git a/src/compiler/glsl/opt_add_neg_to_sub.h > b/src/compiler/glsl/opt_add_neg_to_sub.h > new file mode 100644 > index 0000000..9f97071 > --- /dev/null > +++ b/src/compiler/glsl/opt_add_neg_to_sub.h > @@ -0,0 +1,61 @@ > +/* > + * Copyright © 2016 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. > + */ > + > +#ifndef OPT_ADD_NEG_TO_SUB_H > +#define OPT_ADD_NEG_TO_SUB_H > + > +#include "ir.h" > +#include "ir_hierarchical_visitor.h" > + > +class add_neg_to_sub_visitor : public ir_hierarchical_visitor { > +public: > + add_neg_to_sub_visitor() > + { > + /* empty */ > + } > + > + ir_visitor_status visit_leave(ir_expression *ir) > + { > + if (ir->operation != ir_binop_add) > + return visit_continue; > + > + for (unsigned i = 0; i < 2; i++) { > + ir_expression *const op = ir->operands[i]->as_expression(); > + > + if (op != NULL && op->operation == ir_unop_neg) { > + ir->operation = ir_binop_sub; > + > + /* This ensures that -a + b becomes b - a. */ > + if (i == 0) > + ir->operands[0] = ir->operands[1]; > + > + ir->operands[1] = op->operands[0]; > + break; > + } > + } > + > + return visit_continue; > + } > +}; > + > +#endif /* OPT_ADD_NEG_TO_SUB_H */ > diff --git a/src/compiler/glsl/standalone.cpp > b/src/compiler/glsl/standalone.cpp > index 055c433..07793a9 100644 > --- a/src/compiler/glsl/standalone.cpp > +++ b/src/compiler/glsl/standalone.cpp > @@ -37,6 +37,7 @@ > #include "standalone_scaffolding.h" > #include "standalone.h" > #include "util/string_to_uint_map.h" > +#include "opt_add_neg_to_sub.h" > > static const struct standalone_options *options; > > @@ -441,6 +442,9 @@ standalone_compile_shader(const struct standalone_options > *_options, > if (!shader) > continue; > > + add_neg_to_sub_visitor v; > + visit_list_elements(&v, shader->ir); > + > shader->Program = rzalloc(shader, gl_program); > init_gl_program(shader->Program, shader->Stage); > } > diff --git a/src/compiler/glsl/tests/opt_add_neg_to_sub_test.cpp > b/src/compiler/glsl/tests/opt_add_neg_to_sub_test.cpp > new file mode 100644 > index 0000000..b82e47f > --- /dev/null > +++ b/src/compiler/glsl/tests/opt_add_neg_to_sub_test.cpp > @@ -0,0 +1,210 @@ > +/* > + * Copyright © 2016 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 "ir.h" > +#include "ir_builder.h" > +#include "opt_add_neg_to_sub.h" > + > +using namespace ir_builder; > + > +class add_neg_to_sub : public ::testing::Test { > +public: > + virtual void SetUp(); > + virtual void TearDown(); > + > + exec_list instructions; > + ir_factory *body; > + void *mem_ctx; > + ir_variable *var_a; > + ir_variable *var_b; > + ir_variable *var_c; > + add_neg_to_sub_visitor v; > +}; > + > +void > +add_neg_to_sub::SetUp() > +{ > + mem_ctx = ralloc_context(NULL); > + > + instructions.make_empty(); > + body = new ir_factory(&instructions, mem_ctx); > + > + var_a = new(mem_ctx) ir_variable(glsl_type::float_type, > + "a", > + ir_var_temporary); > + > + var_b = new(mem_ctx) ir_variable(glsl_type::float_type, > + "b", > + ir_var_temporary); > + > + var_c = new(mem_ctx) ir_variable(glsl_type::float_type, > + "c", > + ir_var_temporary); > +} > + > +void > +add_neg_to_sub::TearDown() > +{ > + delete body; > + body = NULL; > + > + ralloc_free(mem_ctx); > + mem_ctx = NULL; > +} > + > +TEST_F(add_neg_to_sub, a_plus_b) > +{ > + body->emit(assign(var_c, add(var_a, var_b))); > + > + visit_list_elements(&v, &instructions); > + > + ASSERT_FALSE(instructions.is_empty()); > + > + ir_instruction *const ir = (ir_instruction *) instructions.pop_head(); > + > + EXPECT_TRUE(instructions.is_empty()); > + > + /* The resulting instruction should be 'c = a + b'. */ > + ir_assignment *const assign = ir->as_assignment(); > + ASSERT_NE((void *)0, assign); > + > + EXPECT_EQ(var_c, assign->lhs->variable_referenced()); > + > + ir_expression *const expr = assign->rhs->as_expression(); > + ASSERT_NE((void *)0, expr); > + EXPECT_EQ(ir_binop_add, expr->operation); > + > + ir_dereference_variable *const deref_a = > + expr->operands[0]->as_dereference_variable(); > + ir_dereference_variable *const deref_b = > + expr->operands[1]->as_dereference_variable(); > + > + ASSERT_NE((void *)0, deref_a); > + EXPECT_EQ(var_a, deref_a->var); > + ASSERT_NE((void *)0, deref_b); > + EXPECT_EQ(var_b, deref_b->var); > +} > + > +TEST_F(add_neg_to_sub, a_plus_neg_b) > +{ > + body->emit(assign(var_c, add(var_a, neg(var_b)))); > + > + visit_list_elements(&v, &instructions); > + > + ASSERT_FALSE(instructions.is_empty()); > + > + ir_instruction *const ir = (ir_instruction *) instructions.pop_head(); > + > + EXPECT_TRUE(instructions.is_empty()); > + > + /* The resulting instruction should be 'c = a - b'. */ > + ir_assignment *const assign = ir->as_assignment(); > + ASSERT_NE((void *)0, assign); > + > + EXPECT_EQ(var_c, assign->lhs->variable_referenced()); > + > + ir_expression *const expr = assign->rhs->as_expression(); > + ASSERT_NE((void *)0, expr); > + EXPECT_EQ(ir_binop_sub, expr->operation); > + > + ir_dereference_variable *const deref_a = > + expr->operands[0]->as_dereference_variable(); > + ir_dereference_variable *const deref_b = > + expr->operands[1]->as_dereference_variable(); > + > + ASSERT_NE((void *)0, deref_a); > + EXPECT_EQ(var_a, deref_a->var); > + ASSERT_NE((void *)0, deref_b); > + EXPECT_EQ(var_b, deref_b->var); > +} > + > +TEST_F(add_neg_to_sub, neg_a_plus_b) > +{ > + body->emit(assign(var_c, add(neg(var_a), var_b))); > + > + visit_list_elements(&v, &instructions); > + > + ASSERT_FALSE(instructions.is_empty()); > + > + ir_instruction *const ir = (ir_instruction *) instructions.pop_head(); > + > + EXPECT_TRUE(instructions.is_empty()); > + > + /* The resulting instruction should be 'c = b - a'. */ > + ir_assignment *const assign = ir->as_assignment(); > + ASSERT_NE((void *)0, assign); > + > + EXPECT_EQ(var_c, assign->lhs->variable_referenced()); > + > + ir_expression *const expr = assign->rhs->as_expression(); > + ASSERT_NE((void *)0, expr); > + EXPECT_EQ(ir_binop_sub, expr->operation); > + > + ir_dereference_variable *const deref_b = > + expr->operands[0]->as_dereference_variable(); > + ir_dereference_variable *const deref_a = > + expr->operands[1]->as_dereference_variable(); > + > + ASSERT_NE((void *)0, deref_a); > + EXPECT_EQ(var_a, deref_a->var); > + ASSERT_NE((void *)0, deref_b); > + EXPECT_EQ(var_b, deref_b->var); > +} > + > +TEST_F(add_neg_to_sub, neg_a_plus_neg_b) > +{ > + body->emit(assign(var_c, add(neg(var_a), neg(var_b)))); > + > + visit_list_elements(&v, &instructions); > + > + ASSERT_FALSE(instructions.is_empty()); > + > + ir_instruction *const ir = (ir_instruction *) instructions.pop_head(); > + > + EXPECT_TRUE(instructions.is_empty()); > + > + /* The resulting instruction should be 'c = -b - a'. */ > + ir_assignment *const assign = ir->as_assignment(); > + ASSERT_NE((void *)0, assign); > + > + EXPECT_EQ(var_c, assign->lhs->variable_referenced()); > + > + ir_expression *const expr = assign->rhs->as_expression(); > + ASSERT_NE((void *)0, expr); > + EXPECT_EQ(ir_binop_sub, expr->operation); > + > + ir_expression *const neg_b = expr->operands[0]->as_expression(); > + ir_dereference_variable *const deref_a = > + expr->operands[1]->as_dereference_variable(); > + > + ASSERT_NE((void *)0, deref_a); > + EXPECT_EQ(var_a, deref_a->var); > + > + ASSERT_NE((void *)0, neg_b); > + > + ir_dereference_variable *const deref_b = > + neg_b->operands[0]->as_dereference_variable(); > + > + ASSERT_NE((void *)0, deref_b); > + EXPECT_EQ(var_b, deref_b->var); > +} >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
