Skip to content

Commit

Permalink
gh-87092: reduce redundancy and repetition in compiler's optimization…
Browse files Browse the repository at this point in the history
… stage (GH-96713)
  • Loading branch information
iritkatriel committed Sep 13, 2022
1 parent 49cceeb commit 6d7a0e0
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 99 deletions.
27 changes: 20 additions & 7 deletions Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
)


Expand Down
2 changes: 0 additions & 2 deletions Lib/test/test_peepholer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
153 changes: 63 additions & 90 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -8271,9 +8271,6 @@ dump_basicblock(const basicblock *b)
#endif


static int
normalize_basic_block(basicblock *bb);

static int
calculate_jump_targets(basicblock *entryblock);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 6d7a0e0

Please sign in to comment.