From 6d7a0e0dd760d4e01e512aad3819c3306c3641a3 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Tue, 13 Sep 2022 13:03:41 +0100 Subject: [PATCH] gh-87092: reduce redundancy and repetition in compiler's optimization stage (GH-96713) --- Lib/test/test_dis.py | 27 +++++-- Lib/test/test_peepholer.py | 2 - Python/compile.c | 153 +++++++++++++++---------------------- 3 files changed, 83 insertions(+), 99 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 48d596027870fc..fc2862c61baadb 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -360,13 +360,13 @@ def bug42562(): --> BINARY_OP 11 (/) POP_TOP -%3d >> LOAD_FAST_CHECK 1 (tb) +%3d LOAD_FAST_CHECK 1 (tb) RETURN_VALUE >> PUSH_EXC_INFO %3d LOAD_GLOBAL 0 (Exception) CHECK_EXC_MATCH - POP_JUMP_IF_FALSE 22 (to 80) + POP_JUMP_IF_FALSE 23 (to 82) STORE_FAST 0 (e) %3d LOAD_FAST 0 (e) @@ -376,7 +376,9 @@ def bug42562(): LOAD_CONST 0 (None) STORE_FAST 0 (e) DELETE_FAST 0 (e) - JUMP_BACKWARD 29 (to 14) + +%3d LOAD_FAST 1 (tb) + RETURN_VALUE >> LOAD_CONST 0 (None) STORE_FAST 0 (e) DELETE_FAST 0 (e) @@ -394,6 +396,7 @@ def bug42562(): TRACEBACK_CODE.co_firstlineno + 5, TRACEBACK_CODE.co_firstlineno + 3, TRACEBACK_CODE.co_firstlineno + 4, + TRACEBACK_CODE.co_firstlineno + 5, TRACEBACK_CODE.co_firstlineno + 3) def _fstring(a, b, c, d): @@ -440,7 +443,7 @@ def _with(c): CALL 2 POP_TOP -%3d >> LOAD_CONST 2 (2) +%3d LOAD_CONST 2 (2) STORE_FAST 2 (y) LOAD_CONST 0 (None) RETURN_VALUE @@ -453,7 +456,11 @@ def _with(c): POP_EXCEPT POP_TOP POP_TOP - JUMP_BACKWARD 13 (to 30) + +%3d LOAD_CONST 2 (2) + STORE_FAST 2 (y) + LOAD_CONST 0 (None) + RETURN_VALUE >> COPY 3 POP_EXCEPT RERAISE 1 @@ -465,6 +472,7 @@ def _with(c): _with.__code__.co_firstlineno + 1, _with.__code__.co_firstlineno + 3, _with.__code__.co_firstlineno + 1, + _with.__code__.co_firstlineno + 3, ) async def _asyncwith(c): @@ -502,7 +510,7 @@ async def _asyncwith(c): JUMP_BACKWARD_NO_INTERRUPT 4 (to 48) >> POP_TOP -%3d >> LOAD_CONST 2 (2) +%3d LOAD_CONST 2 (2) STORE_FAST 2 (y) LOAD_CONST 0 (None) RETURN_VALUE @@ -526,7 +534,11 @@ async def _asyncwith(c): POP_EXCEPT POP_TOP POP_TOP - JUMP_BACKWARD 24 (to 58) + +%3d LOAD_CONST 2 (2) + STORE_FAST 2 (y) + LOAD_CONST 0 (None) + RETURN_VALUE >> COPY 3 POP_EXCEPT RERAISE 1 @@ -538,6 +550,7 @@ async def _asyncwith(c): _asyncwith.__code__.co_firstlineno + 1, _asyncwith.__code__.co_firstlineno + 3, _asyncwith.__code__.co_firstlineno + 1, + _asyncwith.__code__.co_firstlineno + 3, ) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index 5f7e50d4e757af..ab45e3c52a039b 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -344,8 +344,6 @@ def f(x): self.assertEqual(len(returns), 1) self.check_lnotab(f) - @unittest.skip("Following gh-92228 the return has two predecessors " - "and that prevents jump elimination.") def test_elim_jump_to_return(self): # JUMP_FORWARD to RETURN --> RETURN def f(cond, true_value, false_value): diff --git a/Python/compile.c b/Python/compile.c index 33a8679fd8888a..777190526d2575 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -498,7 +498,7 @@ static int compiler_match(struct compiler *, stmt_ty); static int compiler_pattern_subpattern(struct compiler *, pattern_ty, pattern_context *); -static void clean_basic_block(basicblock *bb); +static void remove_redundant_nops(basicblock *bb); static PyCodeObject *assemble(struct compiler *, int addNone); @@ -7364,7 +7364,7 @@ mark_cold(basicblock *entryblock) { for (int i = 0; i < b->b_iused; i++) { struct instr *instr = &b->b_instr[i]; if (is_jump(instr)) { - assert(i == b->b_iused-1); + assert(i == b->b_iused - 1); basicblock *target = b->b_instr[i].i_target; if (!target->b_warm && !target->b_visited) { *sp++ = target; @@ -8271,9 +8271,6 @@ dump_basicblock(const basicblock *b) #endif -static int -normalize_basic_block(basicblock *bb); - static int calculate_jump_targets(basicblock *entryblock); @@ -8325,7 +8322,7 @@ insert_instruction(basicblock *block, int pos, struct instr *instr) { if (basicblock_next_instr(block) < 0) { return -1; } - for (int i = block->b_iused-1; i > pos; i--) { + for (int i = block->b_iused - 1; i > pos; i--) { block->b_instr[i] = block->b_instr[i-1]; } block->b_instr[pos] = *instr; @@ -8488,23 +8485,32 @@ fix_cell_offsets(struct compiler *c, basicblock *entryblock, int *fixedmap) static void propagate_line_numbers(basicblock *entryblock); -static void -eliminate_empty_basic_blocks(cfg_builder *g); - #ifndef NDEBUG static bool no_redundant_jumps(cfg_builder *g) { for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { struct instr *last = basicblock_last_instr(b); if (last != NULL) { - if (last->i_opcode == JUMP || last->i_opcode == JUMP_NO_INTERRUPT) { + if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) { assert(last->i_target != b->b_next); - return false; + if (last->i_target == b->b_next) { + return false; + } } } } return true; } + +static bool +no_empty_basic_blocks(cfg_builder *g) { + for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { + if (b->b_iused == 0) { + return false; + } + } + return true; +} #endif static int @@ -8514,28 +8520,22 @@ remove_redundant_jumps(cfg_builder *g) { * of that jump. If it is, then the jump instruction is redundant and * can be deleted. */ - int removed = 0; + assert(no_empty_basic_blocks(g)); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { struct instr *last = basicblock_last_instr(b); - if (last != NULL) { - assert(!IS_ASSEMBLER_OPCODE(last->i_opcode)); - if (last->i_opcode == JUMP || - last->i_opcode == JUMP_NO_INTERRUPT) { - if (last->i_target == NULL) { - PyErr_SetString(PyExc_SystemError, "jump with NULL target"); - return -1; - } - if (last->i_target == b->b_next) { - assert(b->b_next->b_iused); - last->i_opcode = NOP; - removed++; - } + assert(last != NULL); + assert(!IS_ASSEMBLER_OPCODE(last->i_opcode)); + if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) { + if (last->i_target == NULL) { + PyErr_SetString(PyExc_SystemError, "jump with NULL target"); + return -1; + } + if (last->i_target == b->b_next) { + assert(b->b_next->b_iused); + last->i_opcode = NOP; } } } - if (removed) { - eliminate_empty_basic_blocks(g); - } return 0; } @@ -8643,7 +8643,7 @@ assemble(struct compiler *c, int addNone) goto error; } for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - clean_basic_block(b); + remove_redundant_nops(b); } /* Order of basic blocks must have been determined by now */ @@ -9003,10 +9003,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts) int oparg = inst->i_oparg; int nextop = i+1 < bb->b_iused ? bb->b_instr[i+1].i_opcode : 0; if (HAS_TARGET(inst->i_opcode)) { - /* Skip over empty basic blocks. */ - while (inst->i_target->b_iused == 0) { - inst->i_target = inst->i_target->b_next; - } + assert(inst->i_target->b_iused > 0); target = &inst->i_target->b_instr[0]; assert(!IS_ASSEMBLER_OPCODE(target->i_opcode)); } @@ -9230,50 +9227,36 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts) return -1; } -static bool -basicblock_has_lineno(const basicblock *bb) { - for (int i = 0; i < bb->b_iused; i++) { - if (bb->b_instr[i].i_loc.lineno > 0) { - return true; - } - } - return false; -} - -/* If this block ends with an unconditional jump to an exit block, - * then remove the jump and extend this block with the target. +/* If this block ends with an unconditional jump to a small exit block, then + * remove the jump and extend this block with the target. + * Returns 1 if extended, 0 if no change, and -1 on error. */ static int -extend_block(basicblock *bb) { +inline_small_exit_blocks(basicblock *bb) { struct instr *last = basicblock_last_instr(bb); if (last == NULL) { return 0; } - if (last->i_opcode != JUMP && - last->i_opcode != JUMP_FORWARD && - last->i_opcode != JUMP_BACKWARD) { + if (!IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) { return 0; } - if (basicblock_exits_scope(last->i_target) && last->i_target->b_iused <= MAX_COPY_SIZE) { - basicblock *to_copy = last->i_target; - if (basicblock_has_lineno(to_copy)) { - /* copy only blocks without line number (like implicit 'return None's) */ - return 0; - } + basicblock *target = last->i_target; + if (basicblock_exits_scope(target) && target->b_iused <= MAX_COPY_SIZE) { last->i_opcode = NOP; - for (int i = 0; i < to_copy->b_iused; i++) { + for (int i = 0; i < target->b_iused; i++) { int index = basicblock_next_instr(bb); if (index < 0) { return -1; } - bb->b_instr[index] = to_copy->b_instr[i]; + bb->b_instr[index] = target->b_instr[i]; } + return 1; } return 0; } static void -clean_basic_block(basicblock *bb) { +remove_redundant_nops(basicblock *bb) { /* Remove NOPs when legal to do so. */ int dest = 0; int prev_lineno = -1; @@ -9324,24 +9307,17 @@ clean_basic_block(basicblock *bb) { } static int -normalize_basic_block(basicblock *bb) { - /* Skip over empty blocks. - * Raise SystemError if jump or exit is not last instruction in the block. */ - for (int i = 0; i < bb->b_iused; i++) { - int opcode = bb->b_instr[i].i_opcode; - assert(!IS_ASSEMBLER_OPCODE(opcode)); - int is_jump = IS_JUMP_OPCODE(opcode); - int is_exit = IS_SCOPE_EXIT_OPCODE(opcode); - if (is_exit || is_jump) { - if (i != bb->b_iused-1) { - PyErr_SetString(PyExc_SystemError, "malformed control flow graph."); - return -1; - } - } - if (is_jump) { - /* Skip over empty basic blocks. */ - while (bb->b_instr[i].i_target->b_iused == 0) { - bb->b_instr[i].i_target = bb->b_instr[i].i_target->b_next; +check_cfg(cfg_builder *g) { + for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { + /* Raise SystemError if jump or exit is not last instruction in the block. */ + for (int i = 0; i < b->b_iused; i++) { + int opcode = b->b_instr[i].i_opcode; + assert(!IS_ASSEMBLER_OPCODE(opcode)); + if (IS_TERMINATOR_OPCODE(opcode)) { + if (i != b->b_iused - 1) { + PyErr_SetString(PyExc_SystemError, "malformed control flow graph."); + return -1; + } } } } @@ -9512,25 +9488,25 @@ static int optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache) { assert(PyDict_CheckExact(const_cache)); - for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - if (normalize_basic_block(b)) { - return -1; - } + if (check_cfg(g) < 0) { + return -1; } + eliminate_empty_basic_blocks(g); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - if (extend_block(b) < 0) { + if (inline_small_exit_blocks(b) < 0) { return -1; } } + assert(no_empty_basic_blocks(g)); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { if (optimize_basic_block(const_cache, b, consts)) { return -1; } - clean_basic_block(b); + remove_redundant_nops(b); assert(b->b_predecessors == 0); } for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - if (extend_block(b) < 0) { + if (inline_small_exit_blocks(b) < 0) { return -1; } } @@ -9545,7 +9521,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache) } eliminate_empty_basic_blocks(g); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - clean_basic_block(b); + remove_redundant_nops(b); } if (remove_redundant_jumps(g) < 0) { return -1; @@ -9627,12 +9603,9 @@ duplicate_exits_without_lineno(cfg_builder *g) } } } - /* Eliminate empty blocks */ - for (basicblock *b = entryblock; b != NULL; b = b->b_next) { - while (b->b_next && b->b_next->b_iused == 0) { - b->b_next = b->b_next->b_next; - } - } + + assert(no_empty_basic_blocks(g)); + /* Any remaining reachable exit blocks without line number can only be reached by * fall through, and thus can only have a single predecessor */ for (basicblock *b = entryblock; b != NULL; b = b->b_next) {