From f45a2a93aea0a57cf0aa8ee9ca062fcc42407a44 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Wed, 14 Sep 2016 13:52:42 -0700 Subject: [PATCH] glsl/standalone: Optimize add-of-neg to subtract MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Nicolai Hähnle --- 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 8bfb902..3eac677 100644 --- a/src/compiler/Makefile.glsl.am +++ b/src/compiler/Makefile.glsl.am @@ -72,6 +72,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 f096490..efc6da9 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 +#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); +} -- 2.7.4