2017-05-16 3:31 GMT+02:00 Matt Turner <[email protected]>: > On Mon, Apr 24, 2017 at 4:50 PM, Matt Turner <[email protected]> wrote: >> On Thu, Apr 6, 2017 at 12:49 PM, Thomas Helland >> <[email protected]> wrote: >>> The conditional discard pass follows the same pattern, so merge the >>> two, and avoid running the visitor two times. >>> --- >>> src/compiler/Makefile.sources | 1 - >>> src/compiler/glsl/glsl_parser_extras.cpp | 1 - >>> src/compiler/glsl/ir_optimization.h | 1 - >>> src/compiler/glsl/opt_conditional_discard.cpp | 88 >>> --------------------------- >>> src/compiler/glsl/opt_if_simplification.cpp | 50 +++++++++++---- >>> 5 files changed, 39 insertions(+), 102 deletions(-) >>> delete mode 100644 src/compiler/glsl/opt_conditional_discard.cpp >>> >>> diff --git a/src/compiler/Makefile.sources b/src/compiler/Makefile.sources >>> index 5197c31bb3..817ab3a755 100644 >>> --- a/src/compiler/Makefile.sources >>> +++ b/src/compiler/Makefile.sources >>> @@ -114,7 +114,6 @@ LIBGLSL_FILES = \ >>> glsl/lower_ubo_reference.cpp \ >>> glsl/opt_algebraic.cpp \ >>> glsl/opt_array_splitting.cpp \ >>> - glsl/opt_conditional_discard.cpp \ >>> glsl/opt_constant_folding.cpp \ >>> glsl/opt_constant_propagation.cpp \ >>> glsl/opt_constant_variable.cpp \ >>> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp >>> b/src/compiler/glsl/glsl_parser_extras.cpp >>> index 307e0d6215..fe25c23ccc 100644 >>> --- a/src/compiler/glsl/glsl_parser_extras.cpp >>> +++ b/src/compiler/glsl/glsl_parser_extras.cpp >>> @@ -2139,7 +2139,6 @@ do_common_optimization(exec_list *ir, bool linked, >>> } >>> propagate_invariance(ir); >>> OPT(do_if_simplification, ir); >>> - OPT(opt_conditional_discard, ir); >>> OPT(do_copy_propagation, ir); >>> OPT(do_copy_propagation_elements, ir); >>> >>> diff --git a/src/compiler/glsl/ir_optimization.h >>> b/src/compiler/glsl/ir_optimization.h >>> index 5d57ca85fd..022b7ab12f 100644 >>> --- a/src/compiler/glsl/ir_optimization.h >>> +++ b/src/compiler/glsl/ir_optimization.h >>> @@ -93,7 +93,6 @@ bool ir_constant_fold(ir_rvalue **rvalue); >>> bool do_rebalance_tree(exec_list *instructions); >>> bool do_algebraic(exec_list *instructions, bool native_integers, >>> const struct gl_shader_compiler_options *options); >>> -bool opt_conditional_discard(exec_list *instructions); >>> bool do_constant_folding(exec_list *instructions); >>> bool do_constant_variable(exec_list *instructions); >>> bool do_constant_variable_unlinked(exec_list *instructions); >>> diff --git a/src/compiler/glsl/opt_conditional_discard.cpp >>> b/src/compiler/glsl/opt_conditional_discard.cpp >>> deleted file mode 100644 >>> index 6d8a23460d..0000000000 >>> --- a/src/compiler/glsl/opt_conditional_discard.cpp >>> +++ /dev/null >>> @@ -1,88 +0,0 @@ >>> -/* >>> - * Copyright © 2014 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. >>> - */ >>> - >>> -/** >>> - * \file opt_conditional_discard.cpp >>> - * >>> - * Replace >>> - * >>> - * if (cond) discard; >>> - * >>> - * with >>> - * >>> - * (discard <condition>) >>> - */ >>> - >>> -#include "compiler/glsl_types.h" >>> -#include "ir.h" >>> - >>> -namespace { >>> - >>> -class opt_conditional_discard_visitor : public ir_hierarchical_visitor { >>> -public: >>> - opt_conditional_discard_visitor() >>> - { >>> - progress = false; >>> - } >>> - >>> - ir_visitor_status visit_leave(ir_if *); >>> - >>> - bool progress; >>> -}; >>> - >>> -} /* anonymous namespace */ >>> - >>> -bool >>> -opt_conditional_discard(exec_list *instructions) >>> -{ >>> - opt_conditional_discard_visitor v; >>> - v.run(instructions); >>> - return v.progress; >>> -} >>> - >>> -ir_visitor_status >>> -opt_conditional_discard_visitor::visit_leave(ir_if *ir) >>> -{ >>> - /* Look for "if (...) discard" with no else clause or extra statements. >>> */ >>> - if (ir->then_instructions.is_empty() || >>> - !ir->then_instructions.get_head_raw()->next->is_tail_sentinel() || >>> - !((ir_instruction *) >>> ir->then_instructions.get_head_raw())->as_discard() || >>> - !ir->else_instructions.is_empty()) >>> - return visit_continue; >>> - >>> - /* Move the condition and replace the ir_if with the ir_discard. */ >>> - ir_discard *discard = (ir_discard *) >>> ir->then_instructions.get_head_raw(); >>> - if (!discard->condition) >>> - discard->condition = ir->condition; >>> - else { >>> - void *ctx = ralloc_parent(ir); >>> - discard->condition = new(ctx) ir_expression(ir_binop_logic_and, >>> - ir->condition, >>> - discard->condition); >>> - } >>> - ir->replace_with(discard); >>> - >>> - progress = true; >>> - >>> - return visit_continue; >>> -} >>> diff --git a/src/compiler/glsl/opt_if_simplification.cpp >>> b/src/compiler/glsl/opt_if_simplification.cpp >>> index 05159319ba..64c56d0a2f 100644 >>> --- a/src/compiler/glsl/opt_if_simplification.cpp >>> +++ b/src/compiler/glsl/opt_if_simplification.cpp >>> @@ -44,23 +44,12 @@ public: >>> } >>> >>> ir_visitor_status visit_leave(ir_if *); >>> - ir_visitor_status visit_enter(ir_assignment *); >>> >>> bool made_progress; >>> }; >>> >>> } /* unnamed namespace */ >>> >>> -/* We only care about the top level "if" instructions, so don't >>> - * descend into expressions. >>> - */ >>> -ir_visitor_status >>> -ir_if_simplification_visitor::visit_enter(ir_assignment *ir) >>> -{ >>> - (void) ir; >>> - return visit_continue_with_parent; >>> -} >> >> I don't understand this change. What's going on? >> >> (Everything else in the series looks good. I've made a few trivial >> changes locally -- whitespace mostly) > > As far as I'm aware, the series is blocked waiting on this.
Oh, wow! I'm really sorry for that one. I really need to get my email filter working again, cause I keep missing these mails :/ Quite frankly, I don't understand this change either. Looking back at it now, the only idea that pops to mind is that opt_conditional_discard did not have this hook in it. So the two passes do not follow the exact same pattern, so to not introduce a functional change I removed it. I don't see a good reason for this change though, so I'm currently running some tests with this part in, just to be on the safe side. I'll keep you posted. _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
