Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-87092: reduce redundancy and repetition in compiler's optimization stage #96713

Merged
merged 10 commits into from
Sep 13, 2022
Merged

gh-87092: reduce redundancy and repetition in compiler's optimization stage #96713

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
14f8ac0
use IS_UNCONDITIONAL_JUMP_OPCODE macro. Fix no_redundant_jumps assert…
iritkatriel Sep 6, 2022
d25ce85
rename clean_basic_block --> remove_redundant_nops
iritkatriel Sep 6, 2022
349cc45
rename normalize_basic_block --> check_cfg. Remove the skip-empty-blo…
iritkatriel Sep 6, 2022
42a7525
no need to eliminate empty blocks in duplicate_exits_without_lineno
iritkatriel Sep 9, 2022
8866ee6
rename extend_block --> inline_small_exit_blocks. Make it inline bloc…
iritkatriel Sep 9, 2022
d06fd46
update test_dis
iritkatriel Sep 9, 2022
73a6398
Merge branch 'main' into compiler_assertions
iritkatriel Sep 12, 2022
cf6d362
remove unused variable and redundant NULL check
iritkatriel Sep 12, 2022
d95660a
remove redundant assertion in remove_redundant_jumps
iritkatriel Sep 13, 2022
79d6f53
Merge branch 'main' into compiler_assertions
iritkatriel Sep 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions Lib/test/test_dis.py
View file
Open in desktop
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
View file
Open in desktop
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
128 changes: 52 additions & 76 deletions Python/compile.c
View file
Open in desktop
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 @@ -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 @@ -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 @@ -8519,8 +8525,7 @@ remove_redundant_jumps(cfg_builder *g) {
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 (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
if (last->i_target == NULL) {
PyErr_SetString(PyExc_SystemError, "jump with NULL target");
return -1;
Expand All @@ -8533,9 +8538,7 @@ remove_redundant_jumps(cfg_builder *g) {
}
}
}
if (removed) {
eliminate_empty_basic_blocks(g);
}
assert(no_empty_basic_blocks(g));
sweeneyde marked this conversation as resolved.
Show resolved Hide resolved
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
return 0;
}

Expand Down Expand Up @@ -8643,7 +8646,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 +9006,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 +9230,36 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
return -1;
}

static bool
basicblock_has_lineno(const basicblock *bb) {
markshannon marked this conversation as resolved.
Show resolved Hide resolved
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 +9310,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 +9491,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 +9524,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 +9606,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