[Mesa-dev] [PATCH 3/4] glsl: Merge if-simplification and conditional discard optimization
Matt Turner
mattst88 at gmail.com
Tue May 16 01:31:14 UTC 2017
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.
More information about the mesa-dev
mailing list