I've only skimmed this, but it looks trivial and correct. This patch series is: Reviewed-by: Thomas Helland <thomashellan...@gmail.com>
I like the idea of getting rid of some of the walks over the IR, and have a set of patches sitting locally doing just that. I think I got most of them reviewed, but then it stalled. I've just applied for an account though, so then I won't have to bother others to push my patches for me =) 2017-09-19 4:14 GMT+02:00 Timothy Arceri <tarc...@itsqueeze.com>: > Having this separate just makes the code harder to follow, and > requires an extra walk of the IR. > --- > src/compiler/Makefile.sources | 1 - > src/compiler/glsl/glsl_parser_extras.cpp | 1 - > src/compiler/glsl/loop_analysis.h | 16 ----- > src/compiler/glsl/loop_controls.cpp | 108 > ------------------------------- > src/compiler/glsl/loop_unroll.cpp | 36 ++++++++++- > 5 files changed, 34 insertions(+), 128 deletions(-) > delete mode 100644 src/compiler/glsl/loop_controls.cpp > > diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources > index e03a3f8738c..7146d3db367 100644 > --- a/src/compiler/Makefile.sources > +++ b/src/compiler/Makefile.sources > @@ -80,7 +80,6 @@ LIBGLSL_FILES = \ > glsl/list.h \ > glsl/loop_analysis.cpp \ > glsl/loop_analysis.h \ > - glsl/loop_controls.cpp \ > glsl/loop_unroll.cpp \ > glsl/lower_blend_equation_advanced.cpp \ > glsl/lower_buffer_access.cpp \ > diff --git a/src/compiler/glsl/glsl_parser_extras.cpp > b/src/compiler/glsl/glsl_parser_extras.cpp > index fc56f21a5f0..98151fdb4a4 100644 > --- a/src/compiler/glsl/glsl_parser_extras.cpp > +++ b/src/compiler/glsl/glsl_parser_extras.cpp > @@ -2218,7 +2218,6 @@ do_common_optimization(exec_list *ir, bool linked, > if (options->MaxUnrollIterations) { > loop_state *ls = analyze_loop_variables(ir); > if (ls->loop_found) { > - OPT(set_loop_controls, ir, ls); > OPT(unroll_loops, ir, ls, options); > } > delete ls; > diff --git a/src/compiler/glsl/loop_analysis.h > b/src/compiler/glsl/loop_analysis.h > index e2eff9dbaed..8f824046945 100644 > --- a/src/compiler/glsl/loop_analysis.h > +++ b/src/compiler/glsl/loop_analysis.h > @@ -35,22 +35,6 @@ extern class loop_state * > analyze_loop_variables(exec_list *instructions); > > > -/** > - * Fill in loop control fields > - * > - * Based on analysis of loop variables, this function tries to remove > - * redundant sequences in the loop of the form > - * > - * (if (expression bool ...) (break)) > - * > - * For example, if it is provable that one loop exit condition will > - * always be satisfied before another, the unnecessary exit condition will be > - * removed. > - */ > -extern bool > -set_loop_controls(exec_list *instructions, loop_state *ls); > - > - > extern bool > unroll_loops(exec_list *instructions, loop_state *ls, > const struct gl_shader_compiler_options *options); > diff --git a/src/compiler/glsl/loop_controls.cpp > b/src/compiler/glsl/loop_controls.cpp > deleted file mode 100644 > index ad4aa189411..00000000000 > --- a/src/compiler/glsl/loop_controls.cpp > +++ /dev/null > @@ -1,108 +0,0 @@ > -/* > - * Copyright © 2010 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 <limits.h> > -#include "main/compiler.h" > -#include "compiler/glsl_types.h" > -#include "loop_analysis.h" > -#include "ir_hierarchical_visitor.h" > - > - > -namespace { > - > -class loop_control_visitor : public ir_hierarchical_visitor { > -public: > - loop_control_visitor(loop_state *state) > - { > - this->state = state; > - this->progress = false; > - } > - > - virtual ir_visitor_status visit_leave(ir_loop *ir); > - > - loop_state *state; > - > - bool progress; > -}; > - > -} /* anonymous namespace */ > - > -ir_visitor_status > -loop_control_visitor::visit_leave(ir_loop *ir) > -{ > - loop_variable_state *const ls = this->state->get(ir); > - > - /* If we've entered a loop that hasn't been analyzed, something really, > - * really bad has happened. > - */ > - if (ls == NULL) { > - assert(ls != NULL); > - return visit_continue; > - } > - > - if (ls->limiting_terminator != NULL) { > - /* If the limiting terminator has an iteration count of zero, then > we've > - * proven that the loop cannot run, so delete it. > - */ > - int iterations = ls->limiting_terminator->iterations; > - if (iterations == 0) { > - ir->remove(); > - this->progress = true; > - return visit_continue; > - } > - } > - > - /* Remove the conditional break statements associated with all terminators > - * that are associated with a fixed iteration count, except for the one > - * associated with the limiting terminator--that one needs to stay, since > - * it terminates the loop. Exception: if the loop still has a normative > - * bound, then that terminates the loop, so we don't even need the > limiting > - * terminator. > - */ > - foreach_in_list(loop_terminator, t, &ls->terminators) { > - if (t->iterations < 0) > - continue; > - > - if (t != ls->limiting_terminator) { > - t->ir->remove(); > - > - assert(ls->num_loop_jumps > 0); > - ls->num_loop_jumps--; > - > - this->progress = true; > - } > - } > - > - return visit_continue; > -} > - > - > -bool > -set_loop_controls(exec_list *instructions, loop_state *ls) > -{ > - loop_control_visitor v(ls); > - > - v.run(instructions); > - > - return v.progress; > -} > diff --git a/src/compiler/glsl/loop_unroll.cpp > b/src/compiler/glsl/loop_unroll.cpp > index dbb3fa2fa5c..7eea439454b 100644 > --- a/src/compiler/glsl/loop_unroll.cpp > +++ b/src/compiler/glsl/loop_unroll.cpp > @@ -305,7 +305,6 @@ ir_visitor_status > loop_unroll_visitor::visit_leave(ir_loop *ir) > { > loop_variable_state *const ls = this->state->get(ir); > - int iterations; > > /* If we've entered a loop that hasn't been analyzed, something really, > * really bad has happened. > @@ -315,6 +314,39 @@ loop_unroll_visitor::visit_leave(ir_loop *ir) > return visit_continue; > } > > + if (ls->limiting_terminator != NULL) { > + /* If the limiting terminator has an iteration count of zero, then > we've > + * proven that the loop cannot run, so delete it. > + */ > + int iterations = ls->limiting_terminator->iterations; > + if (iterations == 0) { > + ir->remove(); > + this->progress = true; > + return visit_continue; > + } > + } > + > + /* Remove the conditional break statements associated with all terminators > + * that are associated with a fixed iteration count, except for the one > + * associated with the limiting terminator--that one needs to stay, since > + * it terminates the loop. Exception: if the loop still has a normative > + * bound, then that terminates the loop, so we don't even need the > limiting > + * terminator. > + */ > + foreach_in_list(loop_terminator, t, &ls->terminators) { > + if (t->iterations < 0) > + continue; > + > + if (t != ls->limiting_terminator) { > + t->ir->remove(); > + > + assert(ls->num_loop_jumps > 0); > + ls->num_loop_jumps--; > + > + this->progress = true; > + } > + } > + > if (ls->limiting_terminator == NULL) { > ir_instruction *last_ir = > (ir_instruction *) ir->body_instructions.get_tail(); > @@ -343,7 +375,7 @@ loop_unroll_visitor::visit_leave(ir_loop *ir) > return visit_continue; > } > > - iterations = ls->limiting_terminator->iterations; > + int iterations = ls->limiting_terminator->iterations; > > const int max_iterations = options->MaxUnrollIterations; > > -- > 2.13.5 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev