From: Paul Berry Date: Wed, 29 Jun 2011 17:28:40 +0000 (-0700) Subject: glsl: Add explanatory comments to lower_jumps.cpp. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=e2c748aec5363981a05f21f26a0c4d37ccf6419d;p=mesa.git glsl: Add explanatory comments to lower_jumps.cpp. No functional change. Reviewed-by: Kenneth Graunke --- diff --git a/src/glsl/lower_jumps.cpp b/src/glsl/lower_jumps.cpp index dd2601d1aad..da85c6b49c0 100644 --- a/src/glsl/lower_jumps.cpp +++ b/src/glsl/lower_jumps.cpp @@ -60,12 +60,76 @@ #include #include "ir.h" +/** + * Enum recording the result of analyzing how control flow might exit + * an IR node. + * + * Each possible value of jump_strength indicates a strictly stronger + * guarantee on control flow than the previous value. + * + * The ordering of strengths roughly reflects the way jumps are + * lowered: jumps with higher strength tend to be lowered to jumps of + * lower strength. Accordingly, strength is used as a heuristic to + * determine which lowering to perform first. + * + * This enum is also used by get_jump_strength() to categorize + * instructions as either break, continue, return, or other. When + * used in this fashion, strength_always_clears_execute_flag is not + * used. + * + * The control flow analysis made by this optimization pass makes two + * simplifying assumptions: + * + * - It ignores discard instructions, since they are lowered by a + * separate pass (lower_discard.cpp). + * + * - It assumes it is always possible for control to flow from a loop + * to the instruction immediately following it. Technically, this + * is not true (since all execution paths through the loop might + * jump back to the top, or return from the function). + * + * Both of these simplifying assumtions are safe, since they can never + * cause reachable code to be incorrectly classified as unreachable; + * they can only do the opposite. + */ enum jump_strength { + /** + * Analysis has produced no guarantee on how control flow might + * exit this IR node. It might fall out the bottom (with or + * without clearing the execute flag, if present), or it might + * continue to the top of the innermost enclosing loop, break out + * of it, or return from the function. + */ strength_none, + + /** + * The only way control can fall out the bottom of this node is + * through a code path that clears the execute flag. It might also + * continue to the top of the innermost enclosing loop, break out + * of it, or return from the function. + */ strength_always_clears_execute_flag, + + /** + * Control cannot fall out the bottom of this node. It might + * continue to the top of the innermost enclosing loop, break out + * of it, or return from the function. + */ strength_continue, + + /** + * Control cannot fall out the bottom of this node, or continue the + * top of the innermost enclosing loop. It can only break out of + * it or return from the function. + */ strength_break, + + /** + * Control cannot fall out the bottom of this node, continue to the + * top of the innermost enclosing loop, or break out of it. It can + * only return from the function. + */ strength_return }; @@ -180,6 +244,27 @@ struct function_record }; struct ir_lower_jumps_visitor : public ir_control_flow_visitor { + /* Postconditions: on exit of any visit() function: + * + * ANALYSIS: this->block.min_strength, + * this->block.may_clear_execute_flag, and + * this->loop.may_set_return_flag are updated to reflect the + * characteristics of the visited statement. + * + * DEAD_CODE_ELIMINATION: If this->block.min_strength is not + * strength_none, the visited node is at the end of its exec_list. + * In other words, any unreachable statements that follow the + * visited statement in its exec_list have been removed. + * + * CONTAINED_JUMPS_LOWERED: If the visited statement contains other + * statements, then should_lower_jump() is false for all of the + * return, break, or continue statements it contains. + * + * Note that visiting a jump does not lower it. That is the + * responsibility of the statement (or function signature) that + * contains the jump. + */ + bool progress; struct function_record function; @@ -220,18 +305,57 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor { virtual void visit(class ir_loop_jump * ir) { + /* Eliminate all instructions after each one, since they are + * unreachable. This satisfies the DEAD_CODE_ELIMINATION + * postcondition. + */ truncate_after_instruction(ir); + + /* Set this->block.min_strength based on this instruction. This + * satisfies the ANALYSIS postcondition. It is not necessary to + * update this->block.may_clear_execute_flag or + * this->loop.may_set_return_flag, because an unlowered jump + * instruction can't change any flags. + */ this->block.min_strength = ir->is_break() ? strength_break : strength_continue; + + /* The CONTAINED_JUMPS_LOWERED postcondition is already + * satisfied, because jump statements can't contain other + * statements. + */ } virtual void visit(class ir_return * ir) { + /* Eliminate all instructions after each one, since they are + * unreachable. This satisfies the DEAD_CODE_ELIMINATION + * postcondition. + */ truncate_after_instruction(ir); + + /* Set this->block.min_strength based on this instruction. This + * satisfies the ANALYSIS postcondition. It is not necessary to + * update this->block.may_clear_execute_flag or + * this->loop.may_set_return_flag, because an unlowered return + * instruction can't change any flags. + */ this->block.min_strength = strength_return; + + /* The CONTAINED_JUMPS_LOWERED postcondition is already + * satisfied, because jump statements can't contain other + * statements. + */ } virtual void visit(class ir_discard * ir) { + /* Nothing needs to be done. The ANALYSIS and + * DEAD_CODE_ELIMINATION postconditions are already satisfied, + * because discard statements are ignored by this optimization + * pass. The CONTAINED_JUMPS_LOWERED postcondition is already + * satisfied, because discard statements can't contain other + * statements. + */ } enum jump_strength get_jump_strength(ir_instruction* ir) @@ -304,18 +428,34 @@ struct ir_lower_jumps_visitor : public ir_control_flow_visitor { block_record block_records[2]; ir_jump* jumps[2]; + /* Recursively lower nested jumps. This satisfies the + * CONTAINED_JUMPS_LOWERED postcondition, except in the case of + * unconditional jumps at the end of ir->then_instructions and + * ir->else_instructions, which are handled below. + */ block_records[0] = visit_block(&ir->then_instructions); block_records[1] = visit_block(&ir->else_instructions); retry: /* we get here if we put code after the if inside a branch */ - for(unsigned i = 0; i < 2; ++i) { - exec_list& list = i ? ir->else_instructions : ir->then_instructions; - jumps[i] = 0; - if(!list.is_empty() && get_jump_strength((ir_instruction*)list.get_tail())) - jumps[i] = (ir_jump*)list.get_tail(); - } + /* Determine which of ir->then_instructions and + * ir->else_instructions end with an unconditional jump. + */ + for(unsigned i = 0; i < 2; ++i) { + exec_list& list = i ? ir->else_instructions : ir->then_instructions; + jumps[i] = 0; + if(!list.is_empty() && get_jump_strength((ir_instruction*)list.get_tail())) + jumps[i] = (ir_jump*)list.get_tail(); + } + + /* Loop until we have satisfied the CONTAINED_JUMPS_LOWERED + * postcondition by lowering jumps in both then_instructions and + * else_instructions. + */ for(;;) { + /* Determine the types of the jumps that terminate + * ir->then_instructions and ir->else_instructions. + */ jump_strength jump_strengths[2]; for(unsigned i = 0; i < 2; ++i) { @@ -326,7 +466,12 @@ retry: /* we get here if we put code after the if inside a branch */ jump_strengths[i] = strength_none; } - /* move both jumps out if possible */ + /* If both code paths end in a jump, and the jumps are the + * same, and we are pulling out jumps, replace them with a + * single jump that comes after the if instruction. The new + * jump will be visited next, and it will be lowered if + * necessary by the loop or conditional that encloses it. + */ if(pull_out_jumps && jump_strengths[0] == jump_strengths[1]) { bool unify = true; if(jump_strengths[0] == strength_continue) @@ -344,10 +489,19 @@ retry: /* we get here if we put code after the if inside a branch */ jumps[1]->remove(); this->progress = true; + /* Update jumps[] to reflect the fact that the jumps + * are gone, and update block_records[] to reflect the + * fact that control can now flow to the next + * instruction. + */ jumps[0] = 0; jumps[1] = 0; block_records[0].min_strength = strength_none; block_records[1].min_strength = strength_none; + + /* The CONTAINED_JUMPS_LOWERED postcondition is now + * satisfied, so we can break out of the loop. + */ break; } } @@ -367,9 +521,18 @@ retry: /* we get here if we put code after the if inside a branch */ else if(should_lower[1]) lower = 1; else + /* Neither code path ends in a jump that needs to be + * lowered, so the CONTAINED_JUMPS_LOWERED postcondition + * is satisfied and we can break out of the loop. + */ 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 + */ ir_variable* return_flag = this->function.get_return_flag(); if(!this->function.signature->return_type->is_void()) { ir_variable* return_value = this->function.get_return_value(); @@ -378,29 +541,58 @@ retry: /* we get here if we put code after the if inside a branch */ jumps[lower]->insert_before(new(ir) ir_assignment(new (ir) ir_dereference_variable(return_flag), new (ir) ir_constant(true), NULL)); this->loop.may_set_return_flag = true; 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 + } 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) { - /* We can't lower to an actual continue because that would execute the increment. + /* To lower a break, we create a break flag (if the loop + * doesn't have one already) and add an instruction that + * sets it. * - * In the lowered code, we instead put the break check between the this->loop body and the increment, - * which is impossible with a real continue as defined by the GLSL IR currently. + * Then we proceed as we would for a continue statement + * (set the execute flag to false to prevent the rest of + * the loop body from executing). * - * Smarter options (such as undoing the increment) are possible but it's not worth implementing them, - * because if break is lowered, continue is almost surely lowered too. + * The visit() function for the loop will ensure that the + * break flag is checked after executing the loop body. */ jumps[lower]->insert_before(new(ir) ir_assignment(new (ir) ir_dereference_variable(this->loop.get_break_flag()), new (ir) ir_constant(true), 0)); goto lower_continue; } 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. + * + * Note that this code path gets exercised when lowering + * return statements that are not inside a loop, so + * this->loop must be initialized even outside of loops. + */ ir_variable* execute_flag = this->loop.get_execute_flag(); jumps[lower]->replace_with(new(ir) ir_assignment(new (ir) ir_dereference_variable(execute_flag), new (ir) ir_constant(false), 0)); + /* Note: we must update block_records and jumps to reflect + * the fact that the control path has been altered to an + * instruction that clears the execute flag. + */ jumps[lower] = 0; block_records[lower].min_strength = strength_always_clears_execute_flag; block_records[lower].may_clear_execute_flag = true; @@ -411,6 +603,12 @@ lower_continue: /* move out a jump out if possible */ if(pull_out_jumps) { + /* If one of the branches ends in a jump, and control cannot + * fall out the bottom of the other branch, then we can move + * the jump after the if. + * + * Set move_out to the branch we are moving a jump out of. + */ int move_out = -1; if(jumps[0] && block_records[1].min_strength >= strength_continue) move_out = 0; @@ -421,22 +619,46 @@ lower_continue: { jumps[move_out]->remove(); ir->insert_after(jumps[move_out]); + /* Note: we must update block_records and jumps to reflect + * the fact that the jump has been moved out of the if. + */ jumps[move_out] = 0; block_records[move_out].min_strength = strength_none; this->progress = true; } } + /* Now satisfy the ANALYSIS postcondition by setting + * this->block.min_strength and + * this->block.may_clear_execute_flag based on the + * characteristics of the two branches. + */ if(block_records[0].min_strength < block_records[1].min_strength) this->block.min_strength = block_records[0].min_strength; else this->block.min_strength = block_records[1].min_strength; this->block.may_clear_execute_flag = this->block.may_clear_execute_flag || block_records[0].may_clear_execute_flag || block_records[1].may_clear_execute_flag; + /* Now we need to clean up the instructions that follow the + * if. + * + * If those instructions are unreachable, then satisfy the + * DEAD_CODE_ELIMINATION postcondition by eliminating them. + * Otherwise that postcondition is already satisfied. + */ if(this->block.min_strength) truncate_after_instruction(ir); else if(this->block.may_clear_execute_flag) { + /* If the "if" instruction might clear the execute flag, then + * we need to guard any instructions that follow so that they + * are only executed if the execute flag is set. + * + * If one of the branches of the "if" always clears the + * execute flag, and the other branch never clears it, then + * this is easy: just move all the instructions following the + * "if" into the branch that never clears it. + */ int move_into = -1; if(block_records[0].min_strength && !block_records[1].may_clear_execute_flag) move_into = 1; @@ -451,14 +673,34 @@ lower_continue: if(!next->is_tail_sentinel()) { move_outer_block_inside(ir, list); + /* If any instructions moved, then we need to visit + * them (since they are now inside the "if"). Since + * block_records[move_into] is in its default state + * (see assertion above), we can safely replace + * block_records[move_into] with the result of this + * analysis. + */ exec_list list; list.head = next; block_records[move_into] = visit_block(&list); + /* + * Then we need to re-start our jump lowering, since one + * of the instructions we moved might be a jump that + * needs to be lowered. + */ this->progress = true; goto retry; } } else { + /* If we get here, then the simple case didn't apply; we + * need to actually guard the instructions that follow. + * + * To avoid creating unnecessarily-deep nesting, first + * look through the instructions that follow and unwrap + * any instructions that that are already wrapped in the + * appropriate guard. + */ ir_instruction* ir_after; for(ir_after = (ir_instruction*)ir->get_next(); !ir_after->is_tail_sentinel();) { @@ -479,6 +721,9 @@ lower_continue: this->progress = true; } + /* Then, wrap all the instructions that follow in a single + * guard. + */ if(!ir->get_next()->is_tail_sentinel()) { assert(this->loop.execute_flag); ir_if* if_execute = new(ir) ir_if(new(ir) ir_dereference_variable(this->loop.execute_flag)); @@ -493,29 +738,87 @@ lower_continue: virtual void visit(ir_loop *ir) { + /* Visit the body of the loop, with a fresh data structure in + * this->loop so that the analysis we do here won't bleed into + * enclosing loops. + * + * We assume that all code after a loop is reachable from the + * loop (see comments on enum jump_strength), so the + * DEAD_CODE_ELIMINATION postcondition is automatically + * satisfied, as is the block.min_strength portion of the + * ANALYSIS postcondition. + * + * The block.may_clear_execute_flag portion of the ANALYSIS + * postcondition is automatically satisfied because execute + * flags do not propagate outside of loops. + * + * The loop.may_set_return_flag portion of the ANALYSIS + * postcondition is handled below. + */ ++this->function.nesting_depth; loop_record saved_loop = this->loop; this->loop = loop_record(this->function.signature, ir); + /* Recursively lower nested jumps. This satisfies the + * CONTAINED_JUMPS_LOWERED postcondition, except in the case of + * an unconditional continue or return at the bottom of the + * loop. + */ block_record body = visit_block(&ir->body_instructions); if(body.min_strength >= strength_break) { - /* FINISHME: turn the this->loop into an if, or replace it with its body */ + /* FINISHME: If the min_strength of the loop body is + * strength_break or strength_return, that means that it + * isn't a loop at all, since control flow always leaves the + * body of the loop via break or return. In principle the + * loop could be eliminated in this case. This optimization + * is not implemented yet. + */ } if(this->loop.break_flag) { + /* If a break flag was generated while visiting the body of + * the loop, then at least one break was lowered, so we need + * to generate an if statement at the end of the loop that + * does a "break" if the break flag is set. The break we + * generate won't violate the CONTAINED_JUMPS_LOWERED + * postcondition, because should_lower_jump() always returns + * false for a break that happens at the end of a loop. + */ ir_if* break_if = new(ir) ir_if(new(ir) ir_dereference_variable(this->loop.break_flag)); break_if->then_instructions.push_tail(new(ir) ir_loop_jump(ir_loop_jump::jump_break)); ir->body_instructions.push_tail(break_if); } + /* If the body of the loop may set the return flag, then at + * least one return was lowered to a break, so we need to ensure + * that the return flag is checked after the body of the loop is + * executed. + */ if(this->loop.may_set_return_flag) { assert(this->function.return_flag); + /* Generate the if statement to check the return flag */ ir_if* return_if = new(ir) ir_if(new(ir) ir_dereference_variable(this->function.return_flag)); + /* Note: we also need to propagate the knowledge that the + * return flag may get set to the outer context. This + * satisfies the loop.may_set_return_flag part of the + * ANALYSIS postcondition. + */ saved_loop.may_set_return_flag = true; if(saved_loop.loop) + /* If this loop is nested inside another one, then the if + * statement that we generated should break out of that + * loop if the return flag is set. Caller will lower that + * break statement if necessary. + */ return_if->then_instructions.push_tail(new(ir) ir_loop_jump(ir_loop_jump::jump_break)); else + /* Otherwise, all we need to do is ensure that the + * instructions that follow are only executed if the + * return flag is clear. We can do that by moving those + * instructions into the else clause of the generated if + * statement. + */ move_outer_block_inside(ir, &return_if->else_instructions); ir->insert_after(return_if); } @@ -536,6 +839,11 @@ lower_continue: this->loop = loop_record(ir); assert(!this->loop.loop); + + /* Visit the body of the function to lower any jumps that occur + * in it, except possibly an unconditional return statement at + * the end of it. + */ visit_block(&ir->body); if(this->function.return_value)