From 6d38e85571ea97b570c70cd9de2fc28c03a1c91c Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Jun 2022 19:30:38 +0100 Subject: [PATCH 01/13] optimize_cfg does not require a struct compiler* --- Python/compile.c | 53 ++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 87d9037ea2891b..fb865a04311933 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -1340,8 +1340,9 @@ compiler_add_o(PyObject *dict, PyObject *o) // Merge const *o* recursively and return constant key object. static PyObject* -merge_consts_recursive(struct compiler *c, PyObject *o) +merge_consts_recursive(PyObject *const_cache, PyObject *o) { + PyDict_CheckExact(const_cache); // None and Ellipsis are singleton, and key is the singleton. // No need to merge object and key. if (o == Py_None || o == Py_Ellipsis) { @@ -1355,22 +1356,22 @@ merge_consts_recursive(struct compiler *c, PyObject *o) } // t is borrowed reference - PyObject *t = PyDict_SetDefault(c->c_const_cache, key, key); + PyObject *t = PyDict_SetDefault(const_cache, key, key); if (t != key) { - // o is registered in c_const_cache. Just use it. + // o is registered in const_cache. Just use it. Py_XINCREF(t); Py_DECREF(key); return t; } - // We registered o in c_const_cache. + // We registered o in const_cache. // When o is a tuple or frozenset, we want to merge its // items too. if (PyTuple_CheckExact(o)) { Py_ssize_t len = PyTuple_GET_SIZE(o); for (Py_ssize_t i = 0; i < len; i++) { PyObject *item = PyTuple_GET_ITEM(o, i); - PyObject *u = merge_consts_recursive(c, item); + PyObject *u = merge_consts_recursive(const_cache, item); if (u == NULL) { Py_DECREF(key); return NULL; @@ -1413,7 +1414,7 @@ merge_consts_recursive(struct compiler *c, PyObject *o) PyObject *item; Py_hash_t hash; while (_PySet_NextEntry(o, &pos, &item, &hash)) { - PyObject *k = merge_consts_recursive(c, item); + PyObject *k = merge_consts_recursive(const_cache, item); if (k == NULL) { Py_DECREF(tuple); Py_DECREF(key); @@ -1451,7 +1452,7 @@ merge_consts_recursive(struct compiler *c, PyObject *o) static Py_ssize_t compiler_add_const(struct compiler *c, PyObject *o) { - PyObject *key = merge_consts_recursive(c, o); + PyObject *key = merge_consts_recursive(c->c_const_cache, o); if (key == NULL) { return -1; } @@ -8034,15 +8035,16 @@ compute_code_flags(struct compiler *c) // Merge *obj* with constant cache. // Unlike merge_consts_recursive(), this function doesn't work recursively. static int -merge_const_one(struct compiler *c, PyObject **obj) +merge_const_one(PyObject *const_cache, PyObject **obj) { + PyDict_CheckExact(const_cache); PyObject *key = _PyCode_ConstantKey(*obj); if (key == NULL) { return 0; } // t is borrowed reference - PyObject *t = PyDict_SetDefault(c->c_const_cache, key, key); + PyObject *t = PyDict_SetDefault(const_cache, key, key); Py_DECREF(key); if (t == NULL) { return 0; @@ -8125,7 +8127,7 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist, if (!names) { goto error; } - if (!merge_const_one(c, &names)) { + if (!merge_const_one(c->c_const_cache, &names)) { goto error; } @@ -8133,7 +8135,7 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist, if (consts == NULL) { goto error; } - if (!merge_const_one(c, &consts)) { + if (!merge_const_one(c->c_const_cache, &consts)) { goto error; } @@ -8184,7 +8186,7 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist, goto error; } - if (!merge_const_one(c, &localsplusnames)) { + if (!merge_const_one(c->c_const_cache, &localsplusnames)) { goto error; } con.localsplusnames = localsplusnames; @@ -8249,7 +8251,7 @@ static int normalize_basic_block(basicblock *bb); static int -optimize_cfg(struct compiler *c, struct assembler *a, PyObject *consts); +optimize_cfg(PyObject *const_cache, struct assembler *a, PyObject *consts); static int trim_unused_consts(struct assembler *a, PyObject *consts); @@ -8582,7 +8584,7 @@ assemble(struct compiler *c, int addNone) goto error; } - if (optimize_cfg(c, &a, consts)) { + if (optimize_cfg(c->c_const_cache, &a, consts)) { goto error; } if (duplicate_exits_without_lineno(c)) { @@ -8650,21 +8652,21 @@ assemble(struct compiler *c, int addNone) if (_PyBytes_Resize(&a.a_except_table, a.a_except_table_off) < 0) { goto error; } - if (!merge_const_one(c, &a.a_except_table)) { + if (!merge_const_one(c->c_const_cache, &a.a_except_table)) { goto error; } if (_PyBytes_Resize(&a.a_linetable, a.a_location_off) < 0) { goto error; } - if (!merge_const_one(c, &a.a_linetable)) { + if (!merge_const_one(c->c_const_cache, &a.a_linetable)) { goto error; } if (_PyBytes_Resize(&a.a_bytecode, a.a_offset * sizeof(_Py_CODEUNIT)) < 0) { goto error; } - if (!merge_const_one(c, &a.a_bytecode)) { + if (!merge_const_one(c->c_const_cache, &a.a_bytecode)) { goto error; } @@ -8703,11 +8705,12 @@ get_const_value(int opcode, int oparg, PyObject *co_consts) Called with codestr pointing to the first LOAD_CONST. */ static int -fold_tuple_on_constants(struct compiler *c, +fold_tuple_on_constants(PyObject *const_cache, struct instr *inst, int n, PyObject *consts) { /* Pre-conditions */ + assert(PyDict_CheckExact(const_cache)); assert(PyList_CheckExact(consts)); assert(inst[n].i_opcode == BUILD_TUPLE); assert(inst[n].i_oparg == n); @@ -8732,7 +8735,7 @@ fold_tuple_on_constants(struct compiler *c, } PyTuple_SET_ITEM(newconst, i, constant); } - if (merge_const_one(c, &newconst) == 0) { + if (merge_const_one(const_cache, &newconst) == 0) { Py_DECREF(newconst); return -1; } @@ -8955,8 +8958,9 @@ jump_thread(struct instr *inst, struct instr *target, int opcode) /* Optimization */ static int -optimize_basic_block(struct compiler *c, basicblock *bb, PyObject *consts) +optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts) { + assert(PyDict_CheckExact(const_cache)); assert(PyList_CheckExact(consts)); struct instr nop; nop.i_opcode = NOP; @@ -9063,7 +9067,7 @@ optimize_basic_block(struct compiler *c, basicblock *bb, PyObject *consts) } } if (i >= oparg) { - if (fold_tuple_on_constants(c, inst-oparg, oparg, consts)) { + if (fold_tuple_on_constants(const_cache, inst-oparg, oparg, consts)) { goto error; } } @@ -9415,16 +9419,17 @@ propagate_line_numbers(struct assembler *a) { */ static int -optimize_cfg(struct compiler *c, struct assembler *a, PyObject *consts) +optimize_cfg(PyObject *const_cache, struct assembler *a, PyObject *consts) { + assert(PyDict_CheckExact(const_cache)); for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { - if (optimize_basic_block(c, b, consts)) { + if (optimize_basic_block(const_cache, b, consts)) { return -1; } clean_basic_block(b); assert(b->b_predecessors == 0); } - for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) { + for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { if (extend_block(b)) { return -1; } From db0be36440786abda78e643403d6d0b07bf88cda Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Jun 2022 19:52:48 +0100 Subject: [PATCH 02/13] optimize_cfg does not require a struct assembler* --- Python/compile.c | 56 ++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index fb865a04311933..9d85758da3f375 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8251,10 +8251,10 @@ static int normalize_basic_block(basicblock *bb); static int -optimize_cfg(PyObject *const_cache, struct assembler *a, PyObject *consts); +optimize_cfg(basicblock *entryblock, PyObject *consts, PyObject *const_cache); static int -trim_unused_consts(struct assembler *a, PyObject *consts); +trim_unused_consts(basicblock *entryblock, PyObject *consts); /* Duplicates exit BBs, so that line numbers can be propagated to them */ static int @@ -8462,21 +8462,21 @@ fix_cell_offsets(struct compiler *c, basicblock *entryblock, int *fixedmap) } static void -propagate_line_numbers(struct assembler *a); +propagate_line_numbers(basicblock *entryblock); static void -eliminate_empty_basic_blocks(basicblock *entry); +eliminate_empty_basic_blocks(basicblock *entryblock); static void -remove_redundant_jumps(basicblock *entry) { +remove_redundant_jumps(basicblock *entryblock) { /* 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) { + for (basicblock *b = entryblock; 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)); @@ -8491,7 +8491,7 @@ remove_redundant_jumps(basicblock *entry) { } } if (removed) { - eliminate_empty_basic_blocks(entry); + eliminate_empty_basic_blocks(entryblock); } } @@ -8584,16 +8584,16 @@ assemble(struct compiler *c, int addNone) goto error; } - if (optimize_cfg(c->c_const_cache, &a, consts)) { + if (optimize_cfg(entryblock, consts, c->c_const_cache)) { goto error; } if (duplicate_exits_without_lineno(c)) { return NULL; } - if (trim_unused_consts(&a, consts)) { + if (trim_unused_consts(entryblock, consts)) { goto error; } - propagate_line_numbers(&a); + propagate_line_numbers(entryblock); guarantee_lineno_for_exits(&a, c->u->u_firstlineno); int maxdepth = stackdepth(c, entryblock); if (maxdepth < 0) { @@ -9299,14 +9299,14 @@ normalize_basic_block(basicblock *bb) { } static int -mark_reachable(struct assembler *a) { - basicblock **stack = make_cfg_traversal_stack(a->a_entry); +mark_reachable(basicblock *entryblock) { + basicblock **stack = make_cfg_traversal_stack(entryblock); if (stack == NULL) { return -1; } basicblock **sp = stack; - a->a_entry->b_predecessors = 1; - *sp++ = a->a_entry; + entryblock->b_predecessors = 1; + *sp++ = entryblock; while (sp > stack) { basicblock *b = *(--sp); b->b_visited = 1; @@ -9340,9 +9340,9 @@ mark_reachable(struct assembler *a) { } static void -eliminate_empty_basic_blocks(basicblock *entry) { +eliminate_empty_basic_blocks(basicblock *entryblock) { /* Eliminate empty blocks */ - for (basicblock *b = entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { basicblock *next = b->b_next; if (next) { while (next->b_iused == 0 && next->b_next) { @@ -9351,7 +9351,7 @@ eliminate_empty_basic_blocks(basicblock *entry) { b->b_next = next; } } - for (basicblock *b = entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (b->b_iused == 0) { continue; } @@ -9377,8 +9377,8 @@ eliminate_empty_basic_blocks(basicblock *entry) { * but has no impact on the generated line number events. */ static void -propagate_line_numbers(struct assembler *a) { - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { +propagate_line_numbers(basicblock *entryblock) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (b->b_iused == 0) { continue; } @@ -9419,32 +9419,32 @@ propagate_line_numbers(struct assembler *a) { */ static int -optimize_cfg(PyObject *const_cache, struct assembler *a, PyObject *consts) +optimize_cfg(basicblock *entryblock, PyObject *consts, PyObject *const_cache) { assert(PyDict_CheckExact(const_cache)); - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (optimize_basic_block(const_cache, b, consts)) { return -1; } clean_basic_block(b); assert(b->b_predecessors == 0); } - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (extend_block(b)) { return -1; } } - if (mark_reachable(a)) { + if (mark_reachable(entryblock)) { return -1; } /* Delete unreachable instructions */ - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (b->b_predecessors == 0) { b->b_iused = 0; } } - eliminate_empty_basic_blocks(a->a_entry); - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + eliminate_empty_basic_blocks(entryblock); + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { clean_basic_block(b); } return 0; @@ -9452,13 +9452,13 @@ optimize_cfg(PyObject *const_cache, struct assembler *a, PyObject *consts) // Remove trailing unused constants. static int -trim_unused_consts(struct assembler *a, PyObject *consts) +trim_unused_consts(basicblock *entryblock, PyObject *consts) { assert(PyList_CheckExact(consts)); // The first constant may be docstring; keep it always. int max_const_index = 0; - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { for (int i = 0; i < b->b_iused; i++) { if ((b->b_instr[i].i_opcode == LOAD_CONST || b->b_instr[i].i_opcode == KW_NAMES) && From 82be9a5aa863aeaa54624e04e1a3d3004c9225e4 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Jun 2022 20:08:06 +0100 Subject: [PATCH 03/13] do not pass the whole assembler to functions that need only a_entry --- Python/compile.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 9d85758da3f375..25ccf82d03f7b6 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7728,12 +7728,12 @@ assemble_emit(struct assembler *a, struct instr *i) } static void -normalize_jumps(struct assembler *a) +normalize_jumps(basicblock *entryblock) { - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { b->b_visited = 0; } - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { b->b_visited = 1; if (b->b_iused == 0) { continue; @@ -7787,7 +7787,7 @@ normalize_jumps(struct assembler *a) } static void -assemble_jump_offsets(struct assembler *a, struct compiler *c) +assemble_jump_offsets(basicblock *entryblock, struct compiler *c) { basicblock *b; int bsize, totsize, extended_arg_recompile; @@ -7797,7 +7797,7 @@ assemble_jump_offsets(struct assembler *a, struct compiler *c) Replace block pointer with position in bytecode. */ do { totsize = 0; - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { bsize = blocksize(b); b->b_offset = totsize; totsize += bsize; @@ -7918,17 +7918,17 @@ scan_block_for_local(int target, basicblock *b, bool unsafe_to_start, #undef MAYBE_PUSH static int -add_checks_for_loads_of_unknown_variables(struct assembler *a, +add_checks_for_loads_of_unknown_variables(basicblock *entryblock, struct compiler *c) { - basicblock **stack = make_cfg_traversal_stack(a->a_entry); + basicblock **stack = make_cfg_traversal_stack(entryblock); if (stack == NULL) { return -1; } Py_ssize_t nparams = PyList_GET_SIZE(c->u->u_ste->ste_varnames); int nlocals = (int)PyDict_GET_SIZE(c->u->u_varnames); for (int target = 0; target < nlocals; target++) { - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { b->b_visited = 0; } basicblock **stack_top = stack; @@ -7938,10 +7938,10 @@ add_checks_for_loads_of_unknown_variables(struct assembler *a, // which are the entry block and any DELETE_FAST statements. if (target >= nparams) { // only non-parameter locals start out uninitialized. - *(stack_top++) = a->a_entry; - a->a_entry->b_visited = 1; + *(stack_top++) = entryblock; + entryblock->b_visited = 1; } - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { scan_block_for_local(target, b, false, &stack_top); } @@ -8392,10 +8392,10 @@ insert_prefix_instructions(struct compiler *c, basicblock *entryblock, * The resulting line number may not be correct according to PEP 626, * but should be "good enough", and no worse than in older versions. */ static void -guarantee_lineno_for_exits(struct assembler *a, int firstlineno) { +guarantee_lineno_for_exits(basicblock *entryblock, int firstlineno) { int lineno = firstlineno; assert(lineno > 0); - for (basicblock *b = a->a_entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (b->b_iused == 0) { continue; } @@ -8594,7 +8594,7 @@ assemble(struct compiler *c, int addNone) goto error; } propagate_line_numbers(entryblock); - guarantee_lineno_for_exits(&a, c->u->u_firstlineno); + guarantee_lineno_for_exits(entryblock, c->u->u_firstlineno); int maxdepth = stackdepth(c, entryblock); if (maxdepth < 0) { goto error; @@ -8616,20 +8616,19 @@ assemble(struct compiler *c, int addNone) } remove_redundant_jumps(entryblock); - assert(a.a_entry == entryblock); for (basicblock *b = entryblock; b != NULL; b = b->b_next) { clean_basic_block(b); } /* Order of basic blocks must have been determined by now */ - normalize_jumps(&a); + normalize_jumps(entryblock); - if (add_checks_for_loads_of_unknown_variables(&a, c) < 0) { + if (add_checks_for_loads_of_unknown_variables(entryblock, c) < 0) { goto error; } /* Can't modify the bytecode after computing jump offsets. */ - assemble_jump_offsets(&a, c); + assemble_jump_offsets(entryblock, c); /* Emit code. */ for(b = entryblock; b != NULL; b = b->b_next) { From a2da370c62c2ad240d2ebe12308dd6cbbe38cfc9 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Jun 2022 22:02:10 +0100 Subject: [PATCH 04/13] Create struct assember just before assemble_emit. The other optimizer functions only use the a_entry field which they can get directly --- Python/compile.c | 55 +++++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 25ccf82d03f7b6..b6d3e76a2c43bc 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7007,7 +7007,6 @@ compiler_match(struct compiler *c, stmt_ty s) struct assembler { PyObject *a_bytecode; /* bytes containing bytecode */ PyObject *a_except_table; /* bytes containing exception table */ - basicblock *a_entry; int a_offset; /* offset into bytecode */ int a_except_table_off; /* offset into exception table */ int a_lineno; /* lineno of last emitted instruction */ @@ -7118,7 +7117,7 @@ stackdepth(struct compiler *c, basicblock *entry) } static int -assemble_init(struct assembler *a, int nblocks, int firstlineno) +assemble_init(struct assembler *a, int firstlineno) { memset(a, 0, sizeof(struct assembler)); a->a_lineno = firstlineno; @@ -7138,10 +7137,6 @@ assemble_init(struct assembler *a, int nblocks, int firstlineno) if (a->a_except_table == NULL) { goto error; } - if ((size_t)nblocks > SIZE_MAX / sizeof(basicblock *)) { - PyErr_NoMemory(); - goto error; - } return 1; error: Py_XDECREF(a->a_bytecode); @@ -7529,13 +7524,13 @@ assemble_emit_exception_table_entry(struct assembler *a, int start, int end, bas } static int -assemble_exception_table(struct assembler *a) +assemble_exception_table(struct assembler *a, basicblock *entryblock) { basicblock *b; int ioffset = 0; basicblock *handler = NULL; int start = -1; - for (b = a->a_entry; b != NULL; b = b->b_next) { + for (b = entryblock; b != NULL; b = b->b_next) { ioffset = b->b_offset; for (int i = 0; i < b->b_iused; i++) { struct instr *instr = &b->b_instr[i]; @@ -8498,9 +8493,6 @@ remove_redundant_jumps(basicblock *entryblock) { static PyCodeObject * assemble(struct compiler *c, int addNone) { - basicblock *b, *entryblock; - struct assembler a; - int j, nblocks; PyCodeObject *co = NULL; PyObject *consts = NULL; @@ -8529,15 +8521,6 @@ assemble(struct compiler *c, int addNone) } } - nblocks = 0; - entryblock = NULL; - for (b = c->u->u_blocks; b != NULL; b = b->b_list) { - nblocks++; - entryblock = b; - assert(b->b_warm == 0 && b->b_cold == 0); - } - assert(entryblock != NULL); - assert(PyDict_GET_SIZE(c->u->u_varnames) < INT_MAX); assert(PyDict_GET_SIZE(c->u->u_cellvars) < INT_MAX); assert(PyDict_GET_SIZE(c->u->u_freevars) < INT_MAX); @@ -8552,6 +8535,18 @@ assemble(struct compiler *c, int addNone) goto error; } + int nblocks = 0; + basicblock *entryblock = NULL; + for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) { + nblocks++; + entryblock = b; + } + assert(entryblock != NULL); + if ((size_t)nblocks > SIZE_MAX / sizeof(basicblock *)) { + PyErr_NoMemory(); + goto error; + } + /* Set firstlineno if it wasn't explicitly set. */ if (!c->u->u_firstlineno) { if (entryblock->b_instr && entryblock->b_instr->i_loc.lineno) { @@ -8567,10 +8562,6 @@ assemble(struct compiler *c, int addNone) goto error; } - if (!assemble_init(&a, nblocks, c->u->u_firstlineno)) - goto error; - a.a_entry = entryblock; - int numdropped = fix_cell_offsets(c, entryblock, cellfixedoffsets); PyMem_Free(cellfixedoffsets); // At this point we're done with it. cellfixedoffsets = NULL; @@ -8630,22 +8621,28 @@ assemble(struct compiler *c, int addNone) /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(entryblock, c); + + /* Create assembler */ + struct assembler a; + if (!assemble_init(&a, c->u->u_firstlineno)) + goto error; + /* Emit code. */ - for(b = entryblock; b != NULL; b = b->b_next) { - for (j = 0; j < b->b_iused; j++) + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + for (int j = 0; j < b->b_iused; j++) if (!assemble_emit(&a, &b->b_instr[j])) goto error; } /* Emit location info */ a.a_lineno = c->u->u_firstlineno; - for(b = entryblock; b != NULL; b = b->b_next) { - for (j = 0; j < b->b_iused; j++) + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + for (int j = 0; j < b->b_iused; j++) if (!assemble_emit_location(&a, &b->b_instr[j])) goto error; } - if (!assemble_exception_table(&a)) { + if (!assemble_exception_table(&a, entryblock)) { goto error; } if (_PyBytes_Resize(&a.a_except_table, a.a_except_table_off) < 0) { From c26a2bb96f9d202a82ddb93963c64e05523803ba Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Jun 2022 23:33:16 +0100 Subject: [PATCH 05/13] assemble_jump_offsets doesn't need the compiler --- Python/compile.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index b6d3e76a2c43bc..9677f56f5f1a06 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7782,11 +7782,9 @@ normalize_jumps(basicblock *entryblock) } static void -assemble_jump_offsets(basicblock *entryblock, struct compiler *c) +assemble_jump_offsets(basicblock *entryblock) { - basicblock *b; int bsize, totsize, extended_arg_recompile; - int i; /* Compute the size of each block and fixup jump args. Replace block pointer with position in bytecode. */ @@ -7798,9 +7796,9 @@ assemble_jump_offsets(basicblock *entryblock, struct compiler *c) totsize += bsize; } extended_arg_recompile = 0; - for (b = c->u->u_blocks; b != NULL; b = b->b_list) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { bsize = b->b_offset; - for (i = 0; i < b->b_iused; i++) { + for (int i = 0; i < b->b_iused; i++) { struct instr *instr = &b->b_instr[i]; int isize = instr_size(instr); /* Relative jumps are computed relative to @@ -8619,7 +8617,7 @@ assemble(struct compiler *c, int addNone) } /* Can't modify the bytecode after computing jump offsets. */ - assemble_jump_offsets(entryblock, c); + assemble_jump_offsets(entryblock); /* Create assembler */ From 36d241087e783cf85dd8439b73ff3d4e81a3fa64 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Jun 2022 23:41:42 +0100 Subject: [PATCH 06/13] duplicate_exits_without_lineno can use b_next links to iterate --- Python/compile.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 9677f56f5f1a06..a8b7337e7c0e38 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8251,7 +8251,7 @@ trim_unused_consts(basicblock *entryblock, PyObject *consts); /* Duplicates exit BBs, so that line numbers can be propagated to them */ static int -duplicate_exits_without_lineno(struct compiler *c); +duplicate_exits_without_lineno(basicblock *entryblock, struct compiler *c); static int extend_block(basicblock *bb); @@ -8576,7 +8576,7 @@ assemble(struct compiler *c, int addNone) if (optimize_cfg(entryblock, consts, c->c_const_cache)) { goto error; } - if (duplicate_exits_without_lineno(c)) { + if (duplicate_exits_without_lineno(entryblock, c)) { return NULL; } if (trim_unused_consts(entryblock, consts)) { @@ -9495,11 +9495,11 @@ is_exit_without_lineno(basicblock *b) { * copy the line number from the sole predecessor block. */ static int -duplicate_exits_without_lineno(struct compiler *c) +duplicate_exits_without_lineno(basicblock *entryblock, struct compiler *c) { /* Copy all exit blocks without line number that are targets of a jump. */ - for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (b->b_iused > 0 && is_jump(&b->b_instr[b->b_iused-1])) { basicblock *target = b->b_instr[b->b_iused-1].i_target; if (is_exit_without_lineno(target) && target->b_predecessors > 1) { @@ -9517,14 +9517,14 @@ duplicate_exits_without_lineno(struct compiler *c) } } /* Eliminate empty blocks */ - for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) { + 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; } } /* 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 = c->u->u_blocks; b != NULL; b = b->b_list) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (BB_HAS_FALLTHROUGH(b) && b->b_next && b->b_iused > 0) { if (is_exit_without_lineno(b->b_next)) { assert(b->b_next->b_iused > 0); From 7f06d708c823ef1d4db48aeda105d979b6d1c2c1 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Jun 2022 23:56:38 +0100 Subject: [PATCH 07/13] stackdepth doesn't need the compiler --- Python/compile.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index a8b7337e7c0e38..c6d91ae7411f04 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7047,22 +7047,22 @@ stackdepth_push(basicblock ***sp, basicblock *b, int depth) * cycles in the flow graph have no net effect on the stack depth. */ static int -stackdepth(struct compiler *c, basicblock *entry) +stackdepth(basicblock *entryblock, int code_flags) { - for (basicblock *b = entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { b->b_startdepth = INT_MIN; } - basicblock **stack = make_cfg_traversal_stack(entry); + basicblock **stack = make_cfg_traversal_stack(entryblock); if (!stack) { return -1; } int maxdepth = 0; basicblock **sp = stack; - if (c->u->u_ste->ste_generator || c->u->u_ste->ste_coroutine) { - stackdepth_push(&sp, entry, 1); + if (code_flags & (CO_GENERATOR | CO_COROUTINE | CO_ASYNC_GENERATOR)) { + stackdepth_push(&sp, entryblock, 1); } else { - stackdepth_push(&sp, entry, 0); + stackdepth_push(&sp, entryblock, 0); } while (sp != stack) { @@ -8584,7 +8584,8 @@ assemble(struct compiler *c, int addNone) } propagate_line_numbers(entryblock); guarantee_lineno_for_exits(entryblock, c->u->u_firstlineno); - int maxdepth = stackdepth(c, entryblock); + + int maxdepth = stackdepth(entryblock, code_flags); if (maxdepth < 0) { goto error; } From 5cd8fc94fa9f23de558c5d68c797b5b7fce3a0a2 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 15 Jun 2022 11:00:34 +0100 Subject: [PATCH 08/13] push_cold_blocks_to_end and duplicate_exits_without_lineno do not need the compiler - insert new blocks as b_list successor of the curent block rather than at the head of the list --- Python/compile.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index c6d91ae7411f04..4e395488619e46 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -854,20 +854,26 @@ compiler_set_qualname(struct compiler *c) /* Allocate a new block and return a pointer to it. Returns NULL on error. */ +static basicblock * +new_basicblock() +{ + basicblock *b = (basicblock *)PyObject_Calloc(1, sizeof(basicblock)); + if (b == NULL) { + PyErr_NoMemory(); + return NULL; + } + return b; +} static basicblock * compiler_new_block(struct compiler *c) { - basicblock *b; - struct compiler_unit *u; - - u = c->u; - b = (basicblock *)PyObject_Calloc(1, sizeof(basicblock)); + basicblock *b = new_basicblock(); if (b == NULL) { - PyErr_NoMemory(); return NULL; } /* Extend the singly linked list of blocks with new block. */ + struct compiler_unit *u = c->u; b->b_list = u->u_blocks; u->u_blocks = b; return b; @@ -883,13 +889,25 @@ compiler_use_next_block(struct compiler *c, basicblock *block) } static basicblock * -compiler_copy_block(struct compiler *c, basicblock *block) +new_basicblock_after(basicblock *prev) +{ + basicblock *result = new_basicblock(); + if (result == NULL) { + return NULL; + } + result->b_list = prev->b_list; + prev->b_list = result; + return result; +} + +static basicblock * +copy_basicblock(basicblock *block) { /* Cannot copy a block if it has a fallthrough, since * a block can only have one fallthrough predecessor. */ assert(BB_NO_FALLTHROUGH(block)); - basicblock *result = compiler_new_block(c); + basicblock *result = new_basicblock_after(block); if (result == NULL) { return NULL; } @@ -7385,7 +7403,7 @@ mark_cold(basicblock *entry) { } static int -push_cold_blocks_to_end(struct compiler *c, basicblock *entry, int code_flags) { +push_cold_blocks_to_end(basicblock *entry, int code_flags) { if (entry->b_next == NULL) { /* single basicblock, no need to reorder */ return 0; @@ -7398,7 +7416,7 @@ push_cold_blocks_to_end(struct compiler *c, basicblock *entry, int code_flags) { /* an explicit jump instead of fallthrough */ for (basicblock *b = entry; b != NULL; b = b->b_next) { if (b->b_cold && BB_HAS_FALLTHROUGH(b) && b->b_next && b->b_next->b_warm) { - basicblock *explicit_jump = compiler_new_block(c); + basicblock *explicit_jump = new_basicblock_after(b); if (explicit_jump == NULL) { return -1; } @@ -8251,7 +8269,7 @@ trim_unused_consts(basicblock *entryblock, PyObject *consts); /* Duplicates exit BBs, so that line numbers can be propagated to them */ static int -duplicate_exits_without_lineno(basicblock *entryblock, struct compiler *c); +duplicate_exits_without_lineno(basicblock *entryblock); static int extend_block(basicblock *bb); @@ -8576,7 +8594,7 @@ assemble(struct compiler *c, int addNone) if (optimize_cfg(entryblock, consts, c->c_const_cache)) { goto error; } - if (duplicate_exits_without_lineno(entryblock, c)) { + if (duplicate_exits_without_lineno(entryblock)) { return NULL; } if (trim_unused_consts(entryblock, consts)) { @@ -8601,7 +8619,7 @@ assemble(struct compiler *c, int addNone) } convert_exception_handlers_to_nops(entryblock); - if (push_cold_blocks_to_end(c, entryblock, code_flags) < 0) { + if (push_cold_blocks_to_end(entryblock, code_flags) < 0) { goto error; } @@ -9496,7 +9514,7 @@ is_exit_without_lineno(basicblock *b) { * copy the line number from the sole predecessor block. */ static int -duplicate_exits_without_lineno(basicblock *entryblock, struct compiler *c) +duplicate_exits_without_lineno(basicblock *entryblock) { /* Copy all exit blocks without line number that are targets of a jump. */ @@ -9504,7 +9522,7 @@ duplicate_exits_without_lineno(basicblock *entryblock, struct compiler *c) if (b->b_iused > 0 && is_jump(&b->b_instr[b->b_iused-1])) { basicblock *target = b->b_instr[b->b_iused-1].i_target; if (is_exit_without_lineno(target) && target->b_predecessors > 1) { - basicblock *new_target = compiler_copy_block(c, target); + basicblock *new_target = copy_basicblock(target); if (new_target == NULL) { return -1; } From b1e9f7c3cd8a99eb1f34541c2fffaf160fd47ed0 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 15 Jun 2022 12:52:23 +0100 Subject: [PATCH 09/13] remove 3 unused fields from struct assembler --- Python/compile.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 4e395488619e46..0e4ed78d00326e 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7024,14 +7024,11 @@ compiler_match(struct compiler *c, stmt_ty s) struct assembler { PyObject *a_bytecode; /* bytes containing bytecode */ - PyObject *a_except_table; /* bytes containing exception table */ int a_offset; /* offset into bytecode */ + PyObject *a_except_table; /* bytes containing exception table */ int a_except_table_off; /* offset into exception table */ - int a_lineno; /* lineno of last emitted instruction */ - int a_end_lineno; /* end_lineno of last emitted instruction */ - int a_lineno_start; /* bytecode start offset of current lineno */ - int a_end_lineno_start; /* bytecode start offset of current end_lineno */ /* Location Info */ + int a_lineno; /* lineno of last emitted instruction */ PyObject* a_linetable; /* bytes containing location info */ int a_location_off; /* offset of last written location info frame */ }; @@ -7139,7 +7136,6 @@ assemble_init(struct assembler *a, int firstlineno) { memset(a, 0, sizeof(struct assembler)); a->a_lineno = firstlineno; - a->a_end_lineno = firstlineno; a->a_linetable = NULL; a->a_location_off = 0; a->a_except_table = NULL; From b5f58d18ddebf4bf7d6449044cff71ddb5fb3aa7 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 15 Jun 2022 13:03:20 +0100 Subject: [PATCH 10/13] remove obsolete comment --- Python/compile.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 0e4ed78d00326e..3da9d4d314c8e7 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7013,13 +7013,8 @@ compiler_match(struct compiler *c, stmt_ty s) #undef WILDCARD_CHECK #undef WILDCARD_STAR_CHECK -/* End of the compiler section, beginning of the assembler section */ - -/* do depth-first search of basic block graph, starting with block. - post records the block indices in post-order. - XXX must handle implicit jumps from one block to next -*/ +/* End of the compiler section, beginning of the assembler section */ struct assembler { From c4a125e92af2e97fe8eb0b2dccd33e4ffdc209ba Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 15 Jun 2022 15:46:43 +0100 Subject: [PATCH 11/13] code review followup --- Python/compile.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 3da9d4d314c8e7..49aa1900d4ce1d 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -889,7 +889,7 @@ compiler_use_next_block(struct compiler *c, basicblock *block) } static basicblock * -new_basicblock_after(basicblock *prev) +basicblock_new_b_list_successor(basicblock *prev) { basicblock *result = new_basicblock(); if (result == NULL) { @@ -907,7 +907,7 @@ copy_basicblock(basicblock *block) * a block can only have one fallthrough predecessor. */ assert(BB_NO_FALLTHROUGH(block)); - basicblock *result = new_basicblock_after(block); + basicblock *result = basicblock_new_b_list_successor(block); if (result == NULL) { return NULL; } @@ -1360,7 +1360,7 @@ compiler_add_o(PyObject *dict, PyObject *o) static PyObject* merge_consts_recursive(PyObject *const_cache, PyObject *o) { - PyDict_CheckExact(const_cache); + assert(PyDict_CheckExact(const_cache)); // None and Ellipsis are singleton, and key is the singleton. // No need to merge object and key. if (o == Py_None || o == Py_Ellipsis) { @@ -7407,7 +7407,7 @@ push_cold_blocks_to_end(basicblock *entry, int code_flags) { /* an explicit jump instead of fallthrough */ for (basicblock *b = entry; b != NULL; b = b->b_next) { if (b->b_cold && BB_HAS_FALLTHROUGH(b) && b->b_next && b->b_next->b_warm) { - basicblock *explicit_jump = new_basicblock_after(b); + basicblock *explicit_jump = basicblock_new_b_list_successor(b); if (explicit_jump == NULL) { return -1; } From 39f03c09de82f288f8702a0c087901e85619e9b0 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 15 Jun 2022 16:02:01 +0100 Subject: [PATCH 12/13] entry --> entryblock for consistency --- Python/compile.c | 50 ++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 49aa1900d4ce1d..3946aac8c9943c 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7029,9 +7029,9 @@ struct assembler { }; static basicblock** -make_cfg_traversal_stack(basicblock *entry) { +make_cfg_traversal_stack(basicblock *entryblock) { int nblocks = 0; - for (basicblock *b = entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { b->b_visited = 0; nblocks++; } @@ -7221,8 +7221,8 @@ copy_except_stack(ExceptStack *stack) { } static int -label_exception_targets(basicblock *entry) { - basicblock **todo_stack = make_cfg_traversal_stack(entry); +label_exception_targets(basicblock *entryblock) { + basicblock **todo_stack = make_cfg_traversal_stack(entryblock); if (todo_stack == NULL) { return -1; } @@ -7233,9 +7233,9 @@ label_exception_targets(basicblock *entry) { return -1; } except_stack->depth = 0; - todo_stack[0] = entry; - entry->b_visited = 1; - entry->b_exceptstack = except_stack; + todo_stack[0] = entryblock; + entryblock->b_visited = 1; + entryblock->b_exceptstack = except_stack; basicblock **todo = &todo_stack[1]; basicblock *handler = NULL; while (todo > todo_stack) { @@ -7300,7 +7300,7 @@ label_exception_targets(basicblock *entry) { } } #ifdef Py_DEBUG - for (basicblock *b = entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { assert(b->b_exceptstack == NULL); } #endif @@ -7313,15 +7313,15 @@ label_exception_targets(basicblock *entry) { } static int -mark_warm(basicblock *entry) { - basicblock **stack = make_cfg_traversal_stack(entry); +mark_warm(basicblock *entryblock) { + basicblock **stack = make_cfg_traversal_stack(entryblock); if (stack == NULL) { return -1; } basicblock **sp = stack; - *sp++ = entry; - entry->b_visited = 1; + *sp++ = entryblock; + entryblock->b_visited = 1; while (sp > stack) { basicblock *b = *(--sp); assert(!b->b_except_predecessors); @@ -7344,21 +7344,21 @@ mark_warm(basicblock *entry) { } static int -mark_cold(basicblock *entry) { - for (basicblock *b = entry; b != NULL; b = b->b_next) { +mark_cold(basicblock *entryblock) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { assert(!b->b_cold && !b->b_warm); } - if (mark_warm(entry) < 0) { + if (mark_warm(entryblock) < 0) { return -1; } - basicblock **stack = make_cfg_traversal_stack(entry); + basicblock **stack = make_cfg_traversal_stack(entryblock); if (stack == NULL) { return -1; } basicblock **sp = stack; - for (basicblock *b = entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (b->b_except_predecessors) { assert(b->b_except_predecessors == b->b_predecessors); assert(!b->b_warm); @@ -7394,18 +7394,18 @@ mark_cold(basicblock *entry) { } static int -push_cold_blocks_to_end(basicblock *entry, int code_flags) { - if (entry->b_next == NULL) { +push_cold_blocks_to_end(basicblock *entryblock, int code_flags) { + if (entryblock->b_next == NULL) { /* single basicblock, no need to reorder */ return 0; } - if (mark_cold(entry) < 0) { + if (mark_cold(entryblock) < 0) { return -1; } /* If we have a cold block with fallthrough to a warm block, add */ /* an explicit jump instead of fallthrough */ - for (basicblock *b = entry; b != NULL; b = b->b_next) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { if (b->b_cold && BB_HAS_FALLTHROUGH(b) && b->b_next && b->b_next->b_warm) { basicblock *explicit_jump = basicblock_new_b_list_successor(b); if (explicit_jump == NULL) { @@ -7419,11 +7419,11 @@ push_cold_blocks_to_end(basicblock *entry, int code_flags) { } } - assert(!entry->b_cold); /* First block can't be cold */ + assert(!entryblock->b_cold); /* First block can't be cold */ basicblock *cold_blocks = NULL; basicblock *cold_blocks_tail = NULL; - basicblock *b = entry; + basicblock *b = entryblock; while(b->b_next) { assert(!b->b_cold); while (b->b_next && !b->b_next->b_cold) { @@ -7462,8 +7462,8 @@ push_cold_blocks_to_end(basicblock *entry, int code_flags) { } static void -convert_exception_handlers_to_nops(basicblock *entry) { - for (basicblock *b = entry; b != NULL; b = b->b_next) { +convert_exception_handlers_to_nops(basicblock *entryblock) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { for (int i = 0; i < b->b_iused; i++) { struct instr *instr = &b->b_instr[i]; if (is_block_push(instr) || instr->i_opcode == POP_BLOCK) { From c23357023d3a83ab4234281e83efae429d1f27bd Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 15 Jun 2022 16:45:55 +0000 Subject: [PATCH 13/13] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-06-15-16-45-53.gh-issue-93678.1I_ZT3.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-06-15-16-45-53.gh-issue-93678.1I_ZT3.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-06-15-16-45-53.gh-issue-93678.1I_ZT3.rst b/Misc/NEWS.d/next/Core and Builtins/2022-06-15-16-45-53.gh-issue-93678.1I_ZT3.rst new file mode 100644 index 00000000000000..3c99fd2ecf6633 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-06-15-16-45-53.gh-issue-93678.1I_ZT3.rst @@ -0,0 +1 @@ +Refactor compiler optimisation code so that it no longer needs the ``struct assembler`` and ``struct compiler`` passed around. Instead, each function takes the CFG and other data that it actually needs. This will make it possible to test this code directly.