Skip to content

[WIP] lay out cold blocks at end of function #91804

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

Closed
wants to merge 6 commits into from
Closed

[WIP] lay out cold blocks at end of function #91804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
586abbe
emit except handlers at end of function
iritkatriel Apr 21, 2022
7cee46c
remove redundant jumps after reordering cold blocks
iritkatriel Apr 21, 2022
a1ab27d
add some sanity checks
iritkatriel Apr 22, 2022
5cbffc2
fix test_dis and test_code
iritkatriel Apr 22, 2022
399a898
Merge remote-tracking branch 'upstream/main' into cold_code
iritkatriel May 10, 2022
30c85e3
update test_dis after merge
iritkatriel May 10, 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
6 changes: 3 additions & 3 deletions Lib/test/test_code.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,13 +376,13 @@ def test_co_positions_artificial_instructions(self):
],
[
('RESUME', 0),
("COPY", 3),
("POP_EXCEPT", None),
("RERAISE", 1),
("PUSH_EXC_INFO", None),
("LOAD_CONST", None), # artificial 'None'
("STORE_NAME", "e"), # XX: we know the location for this
("DELETE_NAME", "e"),
("RERAISE", 1),
("COPY", 3),
("POP_EXCEPT", None),
("RERAISE", 1)
]
)
Expand Down
73 changes: 36 additions & 37 deletions Lib/test/test_dis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,14 @@ def bug42562():

%3d LOAD_FAST 1 (tb)
RETURN_VALUE
>> COPY 3
POP_EXCEPT
RERAISE 1
>> PUSH_EXC_INFO

%3d LOAD_GLOBAL 0 (Exception)
CHECK_EXC_MATCH
POP_JUMP_FORWARD_IF_FALSE 18 (to 72)
POP_JUMP_FORWARD_IF_FALSE 18 (to 78)
STORE_FAST 0 (e)

%3d LOAD_FAST 0 (e)
Expand All @@ -392,9 +395,6 @@ def bug42562():
RERAISE 1

%3d >> RERAISE 0
>> COPY 3
POP_EXCEPT
RERAISE 1
ExceptionTable:
""" % (TRACEBACK_CODE.co_firstlineno,
TRACEBACK_CODE.co_firstlineno + 1,
Expand Down Expand Up @@ -1478,40 +1478,39 @@ def _prepare_test_cases():
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=322, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=324, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=326, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=31, argval=392, argrepr='to 392', offset=328, starts_line=None, is_jump_target=True, positions=None),
Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=330, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=4, argval='ZeroDivisionError', argrepr='ZeroDivisionError', offset=332, starts_line=22, is_jump_target=False, positions=None),
Instruction(opname='CHECK_EXC_MATCH', opcode=36, arg=None, argval=None, argrepr='', offset=344, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_JUMP_FORWARD_IF_FALSE', opcode=114, arg=18, argval=384, argrepr='to 384', offset=346, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=348, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=3, argval='print', argrepr='NULL + print', offset=350, starts_line=23, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=9, argval='Here we go, here we go, here we go...', argrepr="'Here we go, here we go, here we go...'", offset=362, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PRECALL', opcode=166, arg=1, argval=1, argrepr='', offset=364, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=1, argval=1, argrepr='', offset=368, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=378, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=380, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='JUMP_FORWARD', opcode=110, arg=4, argval=392, argrepr='to 392', offset=382, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=0, argval=0, argrepr='', offset=384, starts_line=22, is_jump_target=True, positions=None),
Instruction(opname='COPY', opcode=120, arg=3, argval=3, argrepr='', offset=386, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=388, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=1, argval=1, argrepr='', offset=390, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=3, argval='print', argrepr='NULL + print', offset=392, starts_line=28, is_jump_target=True, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=404, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PRECALL', opcode=166, arg=1, argval=1, argrepr='', offset=406, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=1, argval=1, argrepr='', offset=410, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=3, argval='print', argrepr='NULL + print', offset=328, starts_line=28, is_jump_target=True, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=340, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PRECALL', opcode=166, arg=1, argval=1, argrepr='', offset=342, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=1, argval=1, argrepr='', offset=346, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=356, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=358, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=360, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=362, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=3, argval='print', argrepr='NULL + print', offset=364, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=376, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PRECALL', opcode=166, arg=1, argval=1, argrepr='', offset=378, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=1, argval=1, argrepr='', offset=382, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=392, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=0, argval=0, argrepr='', offset=394, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='COPY', opcode=120, arg=3, argval=3, argrepr='', offset=396, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=398, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=1, argval=1, argrepr='', offset=400, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=402, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=4, argval='ZeroDivisionError', argrepr='ZeroDivisionError', offset=404, starts_line=22, is_jump_target=False, positions=None),
Instruction(opname='CHECK_EXC_MATCH', opcode=36, arg=None, argval=None, argrepr='', offset=416, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_JUMP_FORWARD_IF_FALSE', opcode=114, arg=18, argval=456, argrepr='to 456', offset=418, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=420, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=0, argval=None, argrepr='None', offset=422, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_VALUE', opcode=83, arg=None, argval=None, argrepr='', offset=424, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PUSH_EXC_INFO', opcode=35, arg=None, argval=None, argrepr='', offset=426, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=3, argval='print', argrepr='NULL + print', offset=428, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=10, argval="OK, now we're done", argrepr='"OK, now we\'re done"', offset=440, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PRECALL', opcode=166, arg=1, argval=1, argrepr='', offset=442, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=1, argval=1, argrepr='', offset=446, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=456, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=0, argval=0, argrepr='', offset=458, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='COPY', opcode=120, arg=3, argval=3, argrepr='', offset=460, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=462, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=1, argval=1, argrepr='', offset=464, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_GLOBAL', opcode=116, arg=3, argval='print', argrepr='NULL + print', offset=422, starts_line=23, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=9, argval='Here we go, here we go, here we go...', argrepr="'Here we go, here we go, here we go...'", offset=434, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='PRECALL', opcode=166, arg=1, argval=1, argrepr='', offset=436, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=1, argval=1, argrepr='', offset=440, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=450, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=452, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='JUMP_BACKWARD', opcode=140, arg=64, argval=328, argrepr='to 328', offset=454, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=0, argval=0, argrepr='', offset=456, starts_line=22, is_jump_target=True, positions=None),
Instruction(opname='COPY', opcode=120, arg=3, argval=3, argrepr='', offset=458, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_EXCEPT', opcode=89, arg=None, argval=None, argrepr='', offset=460, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RERAISE', opcode=119, arg=1, argval=1, argrepr='', offset=462, starts_line=None, is_jump_target=False, positions=None),
]

# One last piece of inspect fodder to check the default line number handling
Expand Down
111 changes: 85 additions & 26 deletions Python/compile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ typedef struct basicblock_ {
unsigned b_exit : 1;
/* b_return is true if a RETURN_VALUE opcode is inserted. */
unsigned b_return : 1;
/* b_cold is true if this block is not perf critical (like an exception handler) */
unsigned b_cold : 1;
} basicblock;

/* fblockinfo tracks the current frame block.
Expand Down Expand Up @@ -334,6 +336,8 @@ struct compiler_unit {

/* true if we need to create an implicit basicblock before the next instr */
int u_need_new_implicit_block;
/* >0 if we are generating cold code */
int u_cold;
};

/* This struct captures the global state of a compilation.
Expand Down Expand Up @@ -828,6 +832,7 @@ static basicblock *
compiler_use_next_block(struct compiler *c, basicblock *block)
{
assert(block != NULL);
block->b_cold = c->u->u_cold > 0;
c->u->u_curblock->b_next = block;
c->u->u_curblock = block;
c->u->u_need_new_implicit_block = 0;
Expand Down Expand Up @@ -903,6 +908,10 @@ compiler_next_instr(basicblock *b)
return b->b_iused++;
}

#define COLD_VALUE(c) (c)->u->u_cold
#define ENTER_COLD(c) (c)->u->u_cold++;
#define EXIT_COLD(c) (c)->u->u_cold--; assert((c)->u->u_cold >= 0);

/* Set the line number and column offset for the following instructions.

The line number is reset in the following cases:
Expand Down Expand Up @@ -3467,6 +3476,7 @@ compiler_try_except(struct compiler *c, stmt_ty s)
}
ADDOP_JUMP_NOLINE(c, JUMP, end);
n = asdl_seq_LEN(s->v.Try.handlers);
ENTER_COLD(c);
compiler_use_next_block(c, except);

UNSET_LOC(c);
Expand Down Expand Up @@ -3569,6 +3579,7 @@ compiler_try_except(struct compiler *c, stmt_ty s)
ADDOP_I(c, RERAISE, 0);
compiler_use_next_block(c, cleanup);
POP_EXCEPT_AND_RERAISE(c);
EXIT_COLD(c);
compiler_use_next_block(c, end);
return 1;
}
Expand Down Expand Up @@ -7359,6 +7370,36 @@ label_exception_targets(basicblock *entry) {
return -1;
}

static void
push_cold_blocks_to_end(basicblock *entry) {
if (entry->b_next == NULL) {
/* single basicblock, no need to reorder */
return;
}
assert(!entry->b_cold); /* First block can't be cold */
basicblock *tail = entry;
while (tail->b_next) {
assert(tail->b_nofallthrough || (tail->b_cold == tail->b_next->b_cold));
tail = tail->b_next;
}
basicblock *origtail = tail;
basicblock *b = entry;
while(b) {
basicblock *next = b->b_next;
if (next == NULL || next == origtail) {
break;
}
if (next->b_cold) {
b->b_next = next->b_next;
next->b_next = NULL;
tail->b_next = next;
tail = next;
} else {
b = next;
}
}
return;
}

static void
convert_exception_handlers_to_nops(basicblock *entry) {
Expand Down Expand Up @@ -8030,8 +8071,8 @@ static void
dump_basicblock(const basicblock *b)
{
const char *b_return = b->b_return ? "return " : "";
fprintf(stderr, "used: %d, depth: %d, offset: %d %s\n",
b->b_iused, b->b_startdepth, b->b_offset, b_return);
fprintf(stderr, "[%d %p] used: %d, depth: %d, offset: %d %s\n",
b->b_cold, b, b->b_iused, b->b_startdepth, b->b_offset, b_return);
if (b->b_instr) {
int i;
for (i = 0; i < b->b_iused; i++) {
Expand Down Expand Up @@ -8277,6 +8318,38 @@ fix_cell_offsets(struct compiler *c, basicblock *entryblock, int *fixedmap)
static void
propagate_line_numbers(struct assembler *a);

static void
eliminate_empty_basic_blocks(basicblock *entry);


static void
remove_redundant_jumps(basicblock *entry) {
/* If a non-empty block ends with a jump instruction, check if the next
* non-empty block reached through normal flow control is the target
* of that jump. If it is, then the jump instruction is redundant and
* can be deleted.
*/
int removed = 0;
for (basicblock *b = entry; b != NULL; b = b->b_next) {
if (b->b_iused > 0) {
struct instr *b_last_instr = &b->b_instr[b->b_iused - 1];
assert(!IS_ASSEMBLER_OPCODE(b_last_instr->i_opcode));
if (b_last_instr->i_opcode == JUMP ||
b_last_instr->i_opcode == JUMP_NO_INTERRUPT) {
if (b_last_instr->i_target == b->b_next) {
assert(b->b_next->b_iused);
b->b_nofallthrough = 0;
b_last_instr->i_opcode = NOP;
removed++;
}
}
}
}
if (removed) {
eliminate_empty_basic_blocks(entry);
}
}

static PyCodeObject *
assemble(struct compiler *c, int addNone)
{
Expand All @@ -8286,6 +8359,9 @@ assemble(struct compiler *c, int addNone)
PyCodeObject *co = NULL;
PyObject *consts = NULL;

/* Check that ENTER_COLD/EXIT_COLD calls are balanced */
assert(COLD_VALUE(c) == 0);

/* Make sure every block that falls off the end returns None. */
if (!c->u->u_curblock->b_return) {
UNSET_LOC(c);
Expand Down Expand Up @@ -8387,6 +8463,11 @@ assemble(struct compiler *c, int addNone)
goto error;
}
convert_exception_handlers_to_nops(entryblock);

push_cold_blocks_to_end(entryblock);

remove_redundant_jumps(entryblock);

for (basicblock *b = a.a_entry; b != NULL; b = b->b_next) {
clean_basic_block(b);
}
Expand Down Expand Up @@ -9223,30 +9304,8 @@ optimize_cfg(struct compiler *c, struct assembler *a, PyObject *consts)
for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
clean_basic_block(b);
}
/* Delete jump instructions made redundant by previous step. If a non-empty
block ends with a jump instruction, check if the next non-empty block
reached through normal flow control is the target of that jump. If it
is, then the jump instruction is redundant and can be deleted.
*/
int maybe_empty_blocks = 0;
for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) {
if (b->b_iused > 0) {
struct instr *b_last_instr = &b->b_instr[b->b_iused - 1];
assert(!IS_ASSEMBLER_OPCODE(b_last_instr->i_opcode));
if (b_last_instr->i_opcode == JUMP ||
b_last_instr->i_opcode == JUMP_NO_INTERRUPT) {
if (b_last_instr->i_target == b->b_next) {
assert(b->b_next->b_iused);
b->b_nofallthrough = 0;
b_last_instr->i_opcode = NOP;
maybe_empty_blocks = 1;
}
}
}
}
if (maybe_empty_blocks) {
eliminate_empty_basic_blocks(a->a_entry);
}
/* Delete jump instructions made redundant by previous step. */
remove_redundant_jumps(a->a_entry);
return 0;
}

Expand Down