From 6545d35aed73d4395384a93b2387b5ca66ad0e57 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 17 Jan 2024 23:38:40 +0000 Subject: [PATCH 1/5] Move line number propagation before optimization --- Lib/test/test_dis.py | 14 +++-------- Python/flowgraph.c | 60 ++++++++++++++++++++++++++------------------ 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 097f05afdf1517..3ae81b2f5d62b0 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -577,14 +577,10 @@ async def _asyncwith(c): RETURN_CONST 0 (None) %4d L12: CLEANUP_THROW - - -- L13: JUMP_BACKWARD_NO_INTERRUPT 25 (to L5) - -%4d L14: CLEANUP_THROW - - -- L15: JUMP_BACKWARD_NO_INTERRUPT 9 (to L11) - -%4d L16: PUSH_EXC_INFO + L13: JUMP_BACKWARD_NO_INTERRUPT 25 (to L5) + L14: CLEANUP_THROW + L15: JUMP_BACKWARD_NO_INTERRUPT 9 (to L11) + L16: PUSH_EXC_INFO WITH_EXCEPT_START GET_AWAITABLE 2 LOAD_CONST 0 (None) @@ -630,8 +626,6 @@ async def _asyncwith(c): _asyncwith.__code__.co_firstlineno + 1, _asyncwith.__code__.co_firstlineno + 3, _asyncwith.__code__.co_firstlineno + 1, - _asyncwith.__code__.co_firstlineno + 1, - _asyncwith.__code__.co_firstlineno + 1, _asyncwith.__code__.co_firstlineno + 3, ) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 4778f89e19b143..9b7cac2d43915f 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -29,6 +29,7 @@ typedef struct _PyCfgInstruction { int i_opcode; int i_oparg; _PyCompilerSrcLocation i_loc; + unsigned i_loc_propagated : 1; /* location was set by propagate_line_numbers */ struct _PyCfgBasicblock *i_target; /* target block (if jump instruction) */ struct _PyCfgBasicblock *i_except; /* target block when exception is raised */ } cfg_instr; @@ -940,7 +941,10 @@ label_exception_targets(basicblock *entryblock) { /***** CFG optimizations *****/ static int -mark_reachable(basicblock *entryblock) { +remove_unreachable(basicblock *entryblock) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + b->b_predecessors = 0; + } basicblock **stack = make_cfg_traversal_stack(entryblock); if (stack == NULL) { return ERROR; @@ -972,6 +976,14 @@ mark_reachable(basicblock *entryblock) { } } PyMem_Free(stack); + + /* Delete unreachable instructions */ + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + if (b->b_predecessors == 0) { + b->b_iused = 0; + b->b_except_handler = 0; + } + } return SUCCESS; } @@ -1149,13 +1161,15 @@ jump_thread(cfg_instr *inst, cfg_instr *target, int opcode) assert(is_jump(target)); // bpo-45773: If inst->i_target == target->i_target, then nothing actually // changes (and we fall into an infinite loop): + if (inst->i_loc.lineno == -1) assert(inst->i_loc_propagated); + if (target->i_loc.lineno == -1) assert(target->i_loc_propagated); if ((inst->i_loc.lineno == target->i_loc.lineno || - inst->i_loc.lineno == -1 || target->i_loc.lineno == -1) && + inst->i_loc_propagated || target->i_loc_propagated) && inst->i_target != target->i_target) { inst->i_target = target->i_target; inst->i_opcode = opcode; - if (inst->i_loc.lineno == -1) { + if (inst->i_loc_propagated && !target->i_loc_propagated) { inst->i_loc = target->i_loc; } return true; @@ -1714,6 +1728,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts) return ERROR; } +static int resolve_line_numbers(cfg_builder *g, int firstlineno); /* Perform optimizations on a control flow graph. The consts object should still be in list form to allow new constants @@ -1723,41 +1738,31 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts) NOPs. Later those NOPs are removed. */ static int -optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache) +optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache, int firstlineno) { assert(PyDict_CheckExact(const_cache)); RETURN_IF_ERROR(check_cfg(g)); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { RETURN_IF_ERROR(inline_small_exit_blocks(b)); } + RETURN_IF_ERROR(remove_unreachable(g->g_entryblock)); + RETURN_IF_ERROR(resolve_line_numbers(g, firstlineno)); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { RETURN_IF_ERROR(optimize_basic_block(const_cache, b, consts)); - assert(b->b_predecessors == 0); } RETURN_IF_ERROR(remove_redundant_nops_and_pairs(g->g_entryblock)); for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { RETURN_IF_ERROR(inline_small_exit_blocks(b)); } - RETURN_IF_ERROR(mark_reachable(g->g_entryblock)); - - /* Delete unreachable instructions */ - for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - if (b->b_predecessors == 0) { - b->b_iused = 0; - b->b_except_handler = 0; - } - } - for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - remove_redundant_nops(b); - } - RETURN_IF_ERROR(remove_redundant_jumps(g)); + RETURN_IF_ERROR(remove_unreachable(g->g_entryblock)); - for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - remove_redundant_nops(b); + for (int n = 0; n < 2; n++) { + for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { + remove_redundant_nops(b); + } + RETURN_IF_ERROR(remove_redundant_jumps(g)); } - RETURN_IF_ERROR(remove_redundant_jumps(g)); - assert(no_redundant_jumps(g)); return SUCCESS; } @@ -2174,7 +2179,9 @@ push_cold_blocks_to_end(cfg_builder *g) { if (!IS_LABEL(b->b_next->b_label)) { b->b_next->b_label.id = next_lbl++; } - basicblock_addop(explicit_jump, JUMP_NO_INTERRUPT, b->b_next->b_label.id, NO_LOCATION); + cfg_instr *prev_instr = basicblock_last_instr(b); + basicblock_addop(explicit_jump, JUMP_NO_INTERRUPT, b->b_next->b_label.id, + prev_instr->i_loc); explicit_jump->b_cold = 1; explicit_jump->b_next = b->b_next; b->b_next = explicit_jump; @@ -2345,6 +2352,7 @@ propagate_line_numbers(basicblock *entryblock) { for (int i = 0; i < b->b_iused; i++) { if (b->b_instr[i].i_loc.lineno < 0) { b->b_instr[i].i_loc = prev_location; + b->b_instr[i].i_loc_propagated = 1; } else { prev_location = b->b_instr[i].i_loc; @@ -2354,6 +2362,7 @@ propagate_line_numbers(basicblock *entryblock) { if (b->b_next->b_iused > 0) { if (b->b_next->b_instr[0].i_loc.lineno < 0) { b->b_next->b_instr[0].i_loc = prev_location; + b->b_next->b_instr[0].i_loc_propagated = 1; } } } @@ -2362,6 +2371,7 @@ propagate_line_numbers(basicblock *entryblock) { if (target->b_predecessors == 1) { if (target->b_instr[0].i_loc.lineno < 0) { target->b_instr[0].i_loc = prev_location; + target->b_instr[0].i_loc_propagated = 1; } } } @@ -2401,7 +2411,6 @@ resolve_line_numbers(cfg_builder *g, int firstlineno) { RETURN_IF_ERROR(duplicate_exits_without_lineno(g)); propagate_line_numbers(g->g_entryblock); - guarantee_lineno_for_exits(g->g_entryblock, firstlineno); return SUCCESS; } @@ -2417,7 +2426,7 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache, RETURN_IF_ERROR(label_exception_targets(g->g_entryblock)); /** Optimization **/ - RETURN_IF_ERROR(optimize_cfg(g, consts, const_cache)); + RETURN_IF_ERROR(optimize_cfg(g, consts, const_cache, firstlineno)); RETURN_IF_ERROR(remove_unused_consts(g->g_entryblock, consts)); RETURN_IF_ERROR( add_checks_for_loads_of_uninitialized_variables( @@ -2425,6 +2434,7 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache, insert_superinstructions(g); RETURN_IF_ERROR(push_cold_blocks_to_end(g)); + guarantee_lineno_for_exits(g->g_entryblock, firstlineno); RETURN_IF_ERROR(resolve_line_numbers(g, firstlineno)); return SUCCESS; } From 60ed85e1e89f87bf61de5c44e538a6dce9761a06 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 18 Jan 2024 17:36:35 +0000 Subject: [PATCH 2/5] guarantee_lineno_for_exits is not doing anything --- Python/flowgraph.c | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 9b7cac2d43915f..3431da1fbb6bcd 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -505,6 +505,21 @@ no_redundant_jumps(cfg_builder *g) { return true; } +static bool +all_exits_have_lineno(basicblock *entryblock) { + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + for (int i = 0; i < b->b_iused; i++) { + cfg_instr *instr = &b->b_instr[i]; + if (instr->i_opcode == RETURN_VALUE) { + if (instr->i_loc.lineno < 0) { + assert(0); + return false; + } + } + } + } + return true; +} #endif /***** CFG preprocessing (jump targets and exceptions) *****/ @@ -2378,34 +2393,6 @@ propagate_line_numbers(basicblock *entryblock) { } } -/* Make sure that all returns have a line number, even if early passes - * have failed to propagate a correct line number. - * 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(basicblock *entryblock, int firstlineno) { - int lineno = firstlineno; - assert(lineno > 0); - for (basicblock *b = entryblock; b != NULL; b = b->b_next) { - cfg_instr *last = basicblock_last_instr(b); - if (last == NULL) { - continue; - } - if (last->i_loc.lineno < 0) { - if (last->i_opcode == RETURN_VALUE) { - for (int i = 0; i < b->b_iused; i++) { - assert(b->b_instr[i].i_loc.lineno < 0); - - b->b_instr[i].i_loc.lineno = lineno; - } - } - } - else { - lineno = last->i_loc.lineno; - } - } -} - static int resolve_line_numbers(cfg_builder *g, int firstlineno) { @@ -2434,7 +2421,7 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache, insert_superinstructions(g); RETURN_IF_ERROR(push_cold_blocks_to_end(g)); - guarantee_lineno_for_exits(g->g_entryblock, firstlineno); + assert(all_exits_have_lineno(g->g_entryblock)); RETURN_IF_ERROR(resolve_line_numbers(g, firstlineno)); return SUCCESS; } From 73b08aaf25cbcdf8d938a1db0a023f9029ffa130 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 18 Jan 2024 21:19:31 +0000 Subject: [PATCH 3/5] bump magic number --- Lib/importlib/_bootstrap_external.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py index 97858ee83f790f..a4d2b7e0184409 100644 --- a/Lib/importlib/_bootstrap_external.py +++ b/Lib/importlib/_bootstrap_external.py @@ -463,6 +463,7 @@ def _write_atomic(path, data, mode=0o666): # Python 3.13a1 3564 (Removed oparg from YIELD_VALUE, changed oparg values of RESUME) # Python 3.13a1 3565 (Oparg of YIELD_VALUE indicates whether it is in a yield-from) # Python 3.13a1 3566 (Emit JUMP_NO_INTERRUPT instead of JUMP for non-loop no-lineno cases) +# Python 3.13a1 3567 (Reimplement line number propagation by the compiler) # Python 3.14 will start with 3600 @@ -479,7 +480,7 @@ def _write_atomic(path, data, mode=0o666): # Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array # in PC/launcher.c must also be updated. -MAGIC_NUMBER = (3566).to_bytes(2, 'little') + b'\r\n' +MAGIC_NUMBER = (3567).to_bytes(2, 'little') + b'\r\n' _RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c From f612d82ad99178ccab8d8a5914fab7294076a003 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 19 Jan 2024 13:18:17 +0000 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-01-19-13-18-13.gh-issue-114265.7HAi--.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-01-19-13-18-13.gh-issue-114265.7HAi--.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-01-19-13-18-13.gh-issue-114265.7HAi--.rst b/Misc/NEWS.d/next/Core and Builtins/2024-01-19-13-18-13.gh-issue-114265.7HAi--.rst new file mode 100644 index 00000000000000..74affbbd09ffb4 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-01-19-13-18-13.gh-issue-114265.7HAi--.rst @@ -0,0 +1 @@ +Compiler propagates line numbers before optimization, leading to more optimization opportunities and removing the need for the ``guarantee_lineno_for_exits`` hack. From 87fa40de90d64852d25cd8e712a0304eda0bc6d4 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Fri, 19 Jan 2024 14:23:56 +0000 Subject: [PATCH 5/5] add comment and assertion --- Python/flowgraph.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 3431da1fbb6bcd..e84030c87b1b4b 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -2195,6 +2195,10 @@ push_cold_blocks_to_end(cfg_builder *g) { b->b_next->b_label.id = next_lbl++; } cfg_instr *prev_instr = basicblock_last_instr(b); + // b cannot be empty because at the end of an exception handler + // there is always a POP_EXCEPT + RERAISE/RETURN + assert(prev_instr); + basicblock_addop(explicit_jump, JUMP_NO_INTERRUPT, b->b_next->b_label.id, prev_instr->i_loc); explicit_jump->b_cold = 1;