2017-05-16 18:42 GMT+02:00 Ian Romanick <[email protected]>: > On 05/16/2017 05:35 AM, Thomas Helland wrote: >> 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. > > The code that was removed was an optimization. This pass only cares > about if-statements. Since assignments cannot contain if-statements, > don't descend into it. An assignment can also not contain a discard, so > we could probably apply the same optimization to the conditional-discard > pass. >
Yeah, adding it to the conditional discard pass should not be an issue. Added the optimization back, and there's no change in shader-db. Although, my shader-db seems to be segfaulting after the ivy-bridge float64 stuff landed. so I'm not able to run the whole thing. > I'd suggest a first patch that adds the optimization to > opt_conditional_discard.cpp followed by a patch that merges the two > while leaving the optimization intact. > I can do that. Give me a couple hours, and I'll have two new patches on here. >> _______________________________________________ >> mesa-dev mailing list >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
