From cf188a0efb9fcc5c56b822116c995d68077e0396 Mon Sep 17 00:00:00 2001 From: Timothy Arceri Date: Wed, 27 Nov 2024 16:13:30 +1100 Subject: [PATCH] glsl: remove return lowering from glsl ir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need it as nir does it for us anyway. Reviewed-by: Marek Olšák Part-of: --- src/compiler/glsl/glsl_parser_extras.cpp | 2 +- src/compiler/glsl/ir_optimization.h | 2 +- src/compiler/glsl/lower_jumps.cpp | 129 +----------- src/compiler/glsl/test_optpass.cpp | 3 +- src/compiler/glsl/tests/lower_jump_cases.py | 220 +------------------- 5 files changed, 10 insertions(+), 346 deletions(-) diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp index 2ac16a57c87..cbec5971b94 100644 --- a/src/compiler/glsl/glsl_parser_extras.cpp +++ b/src/compiler/glsl/glsl_parser_extras.cpp @@ -2509,7 +2509,7 @@ do_common_optimization(exec_list *ir, bool linked, OPT(do_minmax_prune, ir); OPT(do_rebalance_tree, ir); OPT(do_algebraic, ir, native_integers, options); - OPT(do_lower_jumps, ir, true, false, false, options->EmitNoCont); + OPT(do_lower_jumps, ir, true, options->EmitNoCont); /* If an optimization pass fails to preserve the invariant flag, calling * the pass only once earlier may result in incorrect code generation. Always call diff --git a/src/compiler/glsl/ir_optimization.h b/src/compiler/glsl/ir_optimization.h index d65c44a703b..6d5c7f0fe3d 100644 --- a/src/compiler/glsl/ir_optimization.h +++ b/src/compiler/glsl/ir_optimization.h @@ -45,7 +45,7 @@ bool do_dead_code(exec_list *instructions); bool do_dead_code_local(exec_list *instructions); bool do_dead_code_unlinked(exec_list *instructions); bool opt_flip_matrices(exec_list *instructions); -bool do_lower_jumps(exec_list *instructions, bool pull_out_jumps = true, bool lower_sub_return = true, bool lower_main_return = false, bool lower_continue = false); +bool do_lower_jumps(exec_list *instructions, bool pull_out_jumps = true, bool lower_continue = false); bool do_if_simplification(exec_list *instructions); bool opt_flatten_nested_if_blocks(exec_list *instructions); bool do_mat_op_to_vec(exec_list *instructions); diff --git a/src/compiler/glsl/lower_jumps.cpp b/src/compiler/glsl/lower_jumps.cpp index aec80d7e44b..f4e3ac11f1c 100644 --- a/src/compiler/glsl/lower_jumps.cpp +++ b/src/compiler/glsl/lower_jumps.cpp @@ -199,38 +199,16 @@ struct function_record ir_function_signature* signature; ir_variable* return_flag; /* used to break out of all loops and then jump to the return instruction */ ir_variable* return_value; - bool lower_return; unsigned nesting_depth; - function_record(ir_function_signature* p_signature = 0, - bool lower_return = false) + function_record(ir_function_signature* p_signature = 0) { this->signature = p_signature; this->return_flag = 0; this->return_value = 0; this->nesting_depth = 0; - this->lower_return = lower_return; } - ir_variable* get_return_flag() - { - if(!this->return_flag) { - this->return_flag = new(this->signature) ir_variable(&glsl_type_builtin_bool, "return_flag", ir_var_temporary); - this->signature->body.push_head(new(this->signature) ir_assignment(new(this->signature) ir_dereference_variable(return_flag), new(this->signature) ir_constant(false))); - this->signature->body.push_head(this->return_flag); - } - return this->return_flag; - } - - ir_variable* get_return_value() - { - if(!this->return_value) { - assert(!glsl_type_is_void(this->signature->return_type)); - return_value = new(this->signature) ir_variable(this->signature->return_type, "return_value", ir_var_temporary); - this->signature->body.push_head(this->return_value); - } - return this->return_value; - } }; struct ir_lower_jumps_visitor : public ir_control_flow_visitor { @@ -265,15 +243,11 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor { bool pull_out_jumps; bool lower_continue; - bool lower_sub_return; - bool lower_main_return; ir_lower_jumps_visitor() : progress(false), pull_out_jumps(false), - lower_continue(false), - lower_sub_return(false), - lower_main_return(false) + lower_continue(false) { } @@ -298,43 +272,6 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor { } } - /** - * Insert the instructions necessary to lower a return statement, - * before the given return instruction. - */ - void insert_lowered_return(ir_return *ir) - { - ir_variable* return_flag = this->function.get_return_flag(); - if(!glsl_type_is_void(this->function.signature->return_type)) { - ir_variable* return_value = this->function.get_return_value(); - ir->insert_before( - new(ir) ir_assignment( - new (ir) ir_dereference_variable(return_value), - ir->value)); - } - ir->insert_before( - new(ir) ir_assignment( - new (ir) ir_dereference_variable(return_flag), - new (ir) ir_constant(true))); - this->loop.may_set_return_flag = true; - } - - /** - * If the given instruction is a return, lower it to instructions - * that store the return value (if there is one), set the return - * flag, and then break. - * - * It is safe to pass NULL to this function. - */ - void lower_return_unconditionally(ir_instruction *ir) - { - if (get_jump_strength(ir) != strength_return) { - return; - } - insert_lowered_return((ir_return*)ir); - ir->replace_with(new(ir) ir_loop_jump(ir_loop_jump::jump_break)); - } - virtual void visit(class ir_loop_jump * ir) { /* Eliminate all instructions after each one, since they are @@ -400,9 +337,7 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor { return strength_break; else return strength_continue; - } else if(ir->ir_type == ir_type_return) - return strength_return; - else + } else return strength_none; } @@ -421,13 +356,6 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor { case strength_break: lower = false; break; - case strength_return: - /* never lower return at the end of a this->function */ - if(this->function.nesting_depth == 0 && ir->get_next()->is_tail_sentinel()) - lower = false; - else - lower = this->function.lower_return; - break; } return lower; } @@ -563,40 +491,9 @@ retry: /* we get here if we put code after the if inside a branch */ */ break; - if(jump_strengths[lower] == strength_return) { - /* To lower a return, we create a return flag (if the - * function doesn't have one already) and add instructions - * that: 1. store the return value (if this function has a - * non-void return) and 2. set the return flag - */ - insert_lowered_return((ir_return*)jumps[lower]); - if(this->loop.loop) { - /* If we are in a loop, replace the return instruction - * with a break instruction, and then loop so that the - * break instruction can be lowered if necessary. - */ - ir_loop_jump* lowered = 0; - lowered = new(ir) ir_loop_jump(ir_loop_jump::jump_break); - /* Note: we must update block_records and jumps to - * reflect the fact that the control path has been - * altered from a return to a break. - */ - block_records[lower].min_strength = strength_break; - jumps[lower]->replace_with(lowered); - jumps[lower] = lowered; - } else { - /* If we are not in a loop, we then proceed as we would - * for a continue statement (set the execute flag to - * false to prevent the rest of the function from - * executing). - */ - goto lower_continue; - } - this->progress = true; - } else if(jump_strengths[lower] == strength_break) { + if(jump_strengths[lower] == strength_break) { unreachable("no lowering of breaks any more"); } else if(jump_strengths[lower] == strength_continue) { -lower_continue: /* To lower a continue, we create an execute flag (if the * loop doesn't have one already) and replace the continue * with an instruction that clears it. @@ -797,12 +694,6 @@ lower_continue: ir_last->remove(); } - /* If the loop ends in an unconditional return, and we are - * lowering returns, lower it. - */ - if (this->function.lower_return) - lower_return_unconditionally(ir_last); - if(body.min_strength >= strength_break) { /* FINISHME: If the min_strength of the loop body is * strength_break or strength_return, that means that it @@ -870,15 +761,9 @@ lower_continue: assert(!this->function.signature); assert(!this->loop.loop); - bool lower_return; - if (strcmp(ir->function_name(), "main") == 0) - lower_return = lower_main_return; - else - lower_return = lower_sub_return; - function_record saved_function = this->function; loop_record saved_loop = this->loop; - this->function = function_record(ir, lower_return); + this->function = function_record(ir); this->loop = loop_record(ir); assert(!this->loop.loop); @@ -919,13 +804,11 @@ lower_continue: } /* anonymous namespace */ bool -do_lower_jumps(exec_list *instructions, bool pull_out_jumps, bool lower_sub_return, bool lower_main_return, bool lower_continue) +do_lower_jumps(exec_list *instructions, bool pull_out_jumps, bool lower_continue) { ir_lower_jumps_visitor v; v.pull_out_jumps = pull_out_jumps; v.lower_continue = lower_continue; - v.lower_sub_return = lower_sub_return; - v.lower_main_return = lower_main_return; bool progress_ever = false; do { diff --git a/src/compiler/glsl/test_optpass.cpp b/src/compiler/glsl/test_optpass.cpp index 676d690d9a0..075bf496a4a 100644 --- a/src/compiler/glsl/test_optpass.cpp +++ b/src/compiler/glsl/test_optpass.cpp @@ -75,8 +75,7 @@ do_optimization(struct exec_list *ir, const char *optimization, } else if (sscanf(optimization, "do_lower_jumps ( %d , %d , %d , %d ) ", &int_0, &int_1, &int_2, &int_3) == 4) { - return do_lower_jumps(ir, int_0 != 0, int_1 != 0, int_2 != 0, - int_3 != 0); + return do_lower_jumps(ir, int_0 != 0, int_3 != 0); } else if (strcmp(optimization, "do_if_simplification") == 0) { return do_if_simplification(ir); } else if (strcmp(optimization, "do_mat_op_to_vec") == 0) { diff --git a/src/compiler/glsl/tests/lower_jump_cases.py b/src/compiler/glsl/tests/lower_jump_cases.py index 1236ed69b2f..a51c9bbfc4f 100644 --- a/src/compiler/glsl/tests/lower_jump_cases.py +++ b/src/compiler/glsl/tests/lower_jump_cases.py @@ -287,167 +287,6 @@ def create_test_case(input_sexp, expected_sexp, test_name, return (test_name, optimization, input_str, expected_output) -def test_lower_returns_main(): - """Test that do_lower_jumps respects the lower_main_return flag in deciding - whether to lower returns in the main function. - """ - input_sexp = make_test_case('main', 'void', ( - complex_if('', return_()) - )) - expected_sexp = make_test_case('main', 'void', ( - declare_execute_flag() + - declare_return_flag() + - complex_if('', lowered_return()) - )) - yield create_test_case( - input_sexp, expected_sexp, 'lower_returns_main_true', - lower_main_return=True) - yield create_test_case( - input_sexp, input_sexp, 'lower_returns_main_false', - lower_main_return=False) - -def test_lower_returns_sub(): - """Test that do_lower_jumps respects the lower_sub_return flag in deciding - whether to lower returns in subroutines. - """ - input_sexp = make_test_case('sub', 'void', ( - complex_if('', return_()) - )) - expected_sexp = make_test_case('sub', 'void', ( - declare_execute_flag() + - declare_return_flag() + - complex_if('', lowered_return()) - )) - yield create_test_case( - input_sexp, expected_sexp, 'lower_returns_sub_true', - lower_sub_return=True) - yield create_test_case( - input_sexp, input_sexp, 'lower_returns_sub_false', - lower_sub_return=False) - -def test_lower_returns_1(): - """Test that a void return at the end of a function is eliminated.""" - input_sexp = make_test_case('main', 'void', ( - assign_x('a', const_float(1)) + - return_() - )) - expected_sexp = make_test_case('main', 'void', ( - assign_x('a', const_float(1)) - )) - yield create_test_case( - input_sexp, expected_sexp, 'lower_returns_1', lower_main_return=True) - -def test_lower_returns_2(): - """Test that lowering is not performed on a non-void return at the end of - subroutine. - """ - input_sexp = make_test_case('sub', 'float', ( - assign_x('a', const_float(1)) + - return_(const_float(1)) - )) - yield create_test_case( - input_sexp, input_sexp, 'lower_returns_2', lower_sub_return=True) - -def test_lower_returns_3(): - """Test lowering of returns when there is one nested inside a complex - structure of ifs, and one at the end of a function. - - In this case, the latter return needs to be lowered because it will not be - at the end of the function once the final return is inserted. - """ - input_sexp = make_test_case('sub', 'float', ( - complex_if('', return_(const_float(1))) + - return_(const_float(2)) - )) - expected_sexp = make_test_case('sub', 'float', ( - declare_execute_flag() + - declare_return_value() + - declare_return_flag() + - complex_if('', lowered_return(const_float(1))) + - if_execute_flag(lowered_return(const_float(2))) + - final_return() - )) - yield create_test_case( - input_sexp, expected_sexp, 'lower_returns_3', lower_sub_return=True) - -def test_lower_returns_4(): - """Test that returns are properly lowered when they occur in both branches - of an if-statement. - """ - input_sexp = make_test_case('sub', 'float', ( - simple_if('a', return_(const_float(1)), - return_(const_float(2))) - )) - expected_sexp = make_test_case('sub', 'float', ( - declare_execute_flag() + - declare_return_value() + - declare_return_flag() + - simple_if('a', lowered_return(const_float(1)), - lowered_return(const_float(2))) + - final_return() - )) - yield create_test_case( - input_sexp, expected_sexp, 'lower_returns_4', lower_sub_return=True) - -def test_lower_unified_returns(): - """If both branches of an if statement end in a return, and pull_out_jumps - is True, then those returns should be lifted outside the if and then - properly lowered. - - Verify that this lowering occurs during the same pass as the lowering of - other returns by checking that extra temporary variables aren't generated. - """ - input_sexp = make_test_case('main', 'void', ( - complex_if('a', return_()) + - simple_if('b', simple_if('c', return_(), return_())) - )) - expected_sexp = make_test_case('main', 'void', ( - declare_execute_flag() + - declare_return_flag() + - complex_if('a', lowered_return()) + - if_execute_flag(simple_if('b', (simple_if('c', [], []) + - lowered_return()))) - )) - yield create_test_case( - input_sexp, expected_sexp, 'lower_unified_returns', - lower_main_return=True, pull_out_jumps=True) - -def test_lower_pulled_out_jump(): - doc_string = """If one branch of an if ends in a jump, and control cannot - fall out the bottom of the other branch, and pull_out_jumps is - True, then the jump is lifted outside the if. - - Verify that this lowering occurs during the same pass as the - lowering of other jumps by checking that extra temporary - variables aren't generated. - """ - input_sexp = make_test_case('main', 'void', ( - complex_if('a', return_()) + - loop(simple_if('b', simple_if('c', break_(), continue_()), - return_())) + - assign_x('d', const_float(1)) - )) - # Note: optimization produces two other effects: the break - # gets lifted out of the if statements, and the code after the - # loop gets guarded so that it only executes if the return - # flag is clear. - expected_sexp = make_test_case('main', 'void', ( - declare_execute_flag() + - declare_return_flag() + - complex_if('a', lowered_return()) + - if_execute_flag( - loop(simple_if('b', simple_if('c', [], continue_()), - lowered_return_simple()) + - break_()) + - - if_return_flag(assign_x('return_flag', const_bool(1)) + - assign_x('execute_flag', const_bool(0)), - assign_x('d', const_float(1)))) - )) - yield create_test_case( - input_sexp, expected_sexp, 'lower_pulled_out_jump', - lower_main_return=True, pull_out_jumps=True) - def test_remove_continue_at_end_of_loop(): """Test that a redundant continue-statement at the end of a loop is @@ -462,64 +301,7 @@ def test_remove_continue_at_end_of_loop(): )) yield create_test_case(input_sexp, expected_sexp, 'remove_continue_at_end_of_loop') -def test_lower_return_void_at_end_of_loop(): - """Test that a return of void at the end of a loop is properly lowered.""" - input_sexp = make_test_case('main', 'void', ( - loop(assign_x('a', const_float(1)) + - return_()) + - assign_x('b', const_float(2)) - )) - expected_sexp = make_test_case('main', 'void', ( - declare_execute_flag() + - declare_return_flag() + - loop(assign_x('a', const_float(1)) + - lowered_return_simple() + - break_()) + - if_return_flag(assign_x('return_flag', const_bool(1)) + - assign_x('execute_flag', const_bool(0)), - assign_x('b', const_float(2))) - )) - yield create_test_case( - input_sexp, input_sexp, 'return_void_at_end_of_loop_lower_nothing') - yield create_test_case( - input_sexp, expected_sexp, 'return_void_at_end_of_loop_lower_return', - lower_main_return=True) - - -def test_lower_return_non_void_at_end_of_loop(): - """Test that a non-void return at the end of a loop is properly lowered.""" - input_sexp = make_test_case('sub', 'float', ( - loop(assign_x('a', const_float(1)) + - return_(const_float(2))) + - assign_x('b', const_float(3)) + - return_(const_float(4)) - )) - expected_sexp = make_test_case('sub', 'float', ( - declare_execute_flag() + - declare_return_value() + - declare_return_flag() + - loop(assign_x('a', const_float(1)) + - lowered_return_simple(const_float(2)) + - break_()) + - if_return_flag(assign_x('return_value', '(var_ref return_value)') + - assign_x('return_flag', const_bool(1)) + - assign_x('execute_flag', const_bool(0)), - assign_x('b', const_float(3)) + - lowered_return(const_float(4))) + - final_return() - )) - yield create_test_case( - input_sexp, input_sexp, 'return_non_void_at_end_of_loop_lower_nothing') - yield create_test_case( - input_sexp, expected_sexp, - 'return_non_void_at_end_of_loop_lower_return', lower_sub_return=True) - CASES = [ - test_lower_pulled_out_jump, - test_lower_return_non_void_at_end_of_loop, - test_lower_return_void_at_end_of_loop, - test_lower_returns_1, test_lower_returns_2, test_lower_returns_3, - test_lower_returns_4, test_lower_returns_main, test_lower_returns_sub, - test_lower_unified_returns, test_remove_continue_at_end_of_loop, + test_remove_continue_at_end_of_loop, ]