[Mesa-dev] [PATCH 3/4] glsl: Merge if-simplification and conditional discard optimization
Ian Romanick
idr at freedesktop.org
Tue May 16 16:42:42 UTC 2017
On 05/16/2017 05:35 AM, Thomas Helland wrote:
> 2017-05-16 3:31 GMT+02:00 Matt Turner <mattst88 at gmail.com>:
>> On Mon, Apr 24, 2017 at 4:50 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>> On Thu, Apr 6, 2017 at 12:49 PM, Thomas Helland
>>> <thomashelland90 at gmail.com> 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.
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.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list