From 27102b2de60a81fbdf4022976546f909ba628e4e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 20 Sep 2022 11:44:28 +0100 Subject: [PATCH 01/11] add unit test for const trimming --- Lib/test/test_compile.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index 7c55c7148aec60..cd338a5e375e8a 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -671,10 +671,22 @@ def test_merge_code_attrs(self): self.assertIs(f1.__code__.co_linetable, f2.__code__.co_linetable) + @support.cpython_only + def test_strip_unused_consts(self): + def f(): + "docstring" + if True: + return "used" + else: + return "unused" + + self.assertEqual(f.__code__.co_consts, + ("docstring", True, "used")) + # Stripping unused constants is not a strict requirement for the # Python semantics, it's a more an implementation detail. @support.cpython_only - def test_strip_unused_consts(self): + def test_strip_unused_None(self): # Python 3.10rc1 appended None to co_consts when None is not used # at all. See bpo-45056. def f1(): From 98270301ace259ff3b64a82a381641c61169a300 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 20 Sep 2022 13:53:17 +0100 Subject: [PATCH 02/11] move trim_unused_consts to assembly stage --- Python/compile.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 507fd040a89d7d..605eeb6571ec12 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8644,9 +8644,6 @@ assemble(struct compiler *c, int addNone) if (optimize_cfg(g, consts, c->c_const_cache)) { goto error; } - if (trim_unused_consts(g->g_entryblock, consts)) { - goto error; - } if (duplicate_exits_without_lineno(g) < 0) { goto error; } @@ -8682,6 +8679,9 @@ assemble(struct compiler *c, int addNone) /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(g->g_entryblock); + if (trim_unused_consts(g->g_entryblock, consts)) { + goto error; + } /* Create assembler */ if (!assemble_init(&a, c->u->u_firstlineno)) From 103c9ae910854d9bd6c9e21104e8fdf254ec4165 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 20 Sep 2022 18:09:21 +0100 Subject: [PATCH 03/11] move a few things around --- Python/compile.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 605eeb6571ec12..cdee0e4a5db93f 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8628,10 +8628,7 @@ assemble(struct compiler *c, int addNone) } nlocalsplus -= numdropped; - consts = consts_dict_keys_inorder(c->u->u_consts); - if (consts == NULL) { - goto error; - } + /** Desugaring **/ if (calculate_jump_targets(g->g_entryblock)) { goto error; } @@ -8641,6 +8638,12 @@ assemble(struct compiler *c, int addNone) if (label_exception_targets(g->g_entryblock)) { goto error; } + + /** Optimization **/ + consts = consts_dict_keys_inorder(c->u->u_consts); + if (consts == NULL) { + goto error; + } if (optimize_cfg(g, consts, c->c_const_cache)) { goto error; } @@ -8665,6 +8668,7 @@ assemble(struct compiler *c, int addNone) remove_redundant_nops(b); } + /** Assembly **/ /* Order of basic blocks must have been determined by now */ if (normalize_jumps(g) < 0) { goto error; @@ -9599,12 +9603,14 @@ is_exit_without_lineno(basicblock *b) { static int duplicate_exits_without_lineno(cfg_builder *g) { + assert(no_empty_basic_blocks(g)); /* Copy all exit blocks without line number that are targets of a jump. */ basicblock *entryblock = g->g_entryblock; for (basicblock *b = entryblock; b != NULL; b = b->b_next) { struct instr *last = basicblock_last_instr(b); - if (last != NULL && is_jump(last)) { + assert(last != NULL); + if (is_jump(last)) { basicblock *target = last->i_target; if (is_exit_without_lineno(target) && target->b_predecessors > 1) { basicblock *new_target = copy_basicblock(g, target); @@ -9621,8 +9627,6 @@ duplicate_exits_without_lineno(cfg_builder *g) } } - 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) { From bab900a5be9cea8e351820e7c4247bf345ee60e2 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 20 Sep 2022 18:33:48 +0100 Subject: [PATCH 04/11] remove redundant assert and move another up to just after the value was calculated --- Python/compile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index cdee0e4a5db93f..2fed746d1a4fe9 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7080,15 +7080,14 @@ stackdepth(basicblock *entryblock, int code_flags) if (new_depth > maxdepth) { maxdepth = new_depth; } - assert(depth >= 0); /* invalid code or bug in stackdepth() */ if (HAS_TARGET(instr->i_opcode)) { effect = stack_effect(instr->i_opcode, instr->i_oparg, 1); assert(effect != PY_INVALID_STACK_EFFECT); int target_depth = depth + effect; + assert(target_depth >= 0); /* invalid code or bug in stackdepth() */ if (target_depth > maxdepth) { maxdepth = target_depth; } - assert(target_depth >= 0); /* invalid code or bug in stackdepth() */ stackdepth_push(&sp, instr->i_target, target_depth); } depth = new_depth; From 9736e75ec237973802dbe7aae44a001a443edb59 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 21 Sep 2022 07:31:29 +0100 Subject: [PATCH 05/11] move push_cold_blocks_to_end up --- Python/compile.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 2fed746d1a4fe9..3dcb5b4b0e30df 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8649,6 +8649,9 @@ assemble(struct compiler *c, int addNone) if (duplicate_exits_without_lineno(g) < 0) { goto error; } + if (push_cold_blocks_to_end(g, code_flags) < 0) { + goto error; + } propagate_line_numbers(g->g_entryblock); guarantee_lineno_for_exits(g->g_entryblock, c->u->u_firstlineno); @@ -8660,9 +8663,6 @@ assemble(struct compiler *c, int addNone) convert_exception_handlers_to_nops(g->g_entryblock); - if (push_cold_blocks_to_end(g, code_flags) < 0) { - goto error; - } for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { remove_redundant_nops(b); } From eb6cd7614da0c144d811e2ed93c13db3a44c2ea4 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 29 Sep 2022 11:23:03 +0100 Subject: [PATCH 06/11] reduce repetition: create basicblock_append_instructions and call it from two places --- Python/compile.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 3dcb5b4b0e30df..a9372de62d9619 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -912,6 +912,19 @@ cfg_builder_use_label(cfg_builder *g, jump_target_label lbl) return cfg_builder_maybe_start_new_block(g); } +static inline int +basicblock_append_instructions(basicblock *target, basicblock *source) +{ + for (int i = 0; i < source->b_iused; i++) { + int n = basicblock_next_instr(target); + if (n < 0) { + return -1; + } + target->b_instr[n] = source->b_instr[i]; + } + return 0; +} + static basicblock * copy_basicblock(cfg_builder *g, basicblock *block) { @@ -923,12 +936,8 @@ copy_basicblock(cfg_builder *g, basicblock *block) if (result == NULL) { return NULL; } - for (int i = 0; i < block->b_iused; i++) { - int n = basicblock_next_instr(result); - if (n < 0) { - return NULL; - } - result->b_instr[n] = block->b_instr[i]; + if (basicblock_append_instructions(result, block) < 0) { + return NULL; } return result; } @@ -9268,12 +9277,8 @@ inline_small_exit_blocks(basicblock *bb) { 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 < target->b_iused; i++) { - int index = basicblock_next_instr(bb); - if (index < 0) { - return -1; - } - bb->b_instr[index] = target->b_instr[i]; + if (basicblock_append_instructions(bb, target) < 0) { + return -1; } return 1; } From fa0b95d728a1eef9c6e9cd7dc03df82c60a7dc7d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 29 Sep 2022 12:36:55 +0100 Subject: [PATCH 07/11] move remove_redundant_nops call into convert_exception_handlers_to_nops. rename calculate_jump_targets --> translate_jump_labels_to_targets --- Python/compile.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index a9372de62d9619..094810e5daba5a 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7495,6 +7495,9 @@ convert_exception_handlers_to_nops(basicblock *entryblock) { } } } + for (basicblock *b = entryblock; b != NULL; b = b->b_next) { + remove_redundant_nops(b); + } } static inline void @@ -8299,7 +8302,7 @@ dump_basicblock(const basicblock *b) static int -calculate_jump_targets(basicblock *entryblock); +translate_jump_labels_to_targets(basicblock *entryblock); static int optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache); @@ -8637,7 +8640,7 @@ assemble(struct compiler *c, int addNone) nlocalsplus -= numdropped; /** Desugaring **/ - if (calculate_jump_targets(g->g_entryblock)) { + if (translate_jump_labels_to_targets(g->g_entryblock)) { goto error; } if (mark_except_handlers(g->g_entryblock) < 0) { @@ -8655,14 +8658,18 @@ assemble(struct compiler *c, int addNone) if (optimize_cfg(g, consts, c->c_const_cache)) { goto error; } + + /** line numbers (TODO: move this to desugaring stage) */ if (duplicate_exits_without_lineno(g) < 0) { goto error; } + propagate_line_numbers(g->g_entryblock); + guarantee_lineno_for_exits(g->g_entryblock, c->u->u_firstlineno); + + /** Assembly **/ if (push_cold_blocks_to_end(g, code_flags) < 0) { goto error; } - propagate_line_numbers(g->g_entryblock); - guarantee_lineno_for_exits(g->g_entryblock, c->u->u_firstlineno); int maxdepth = stackdepth(g->g_entryblock, code_flags); if (maxdepth < 0) { @@ -8672,16 +8679,12 @@ assemble(struct compiler *c, int addNone) convert_exception_handlers_to_nops(g->g_entryblock); - for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - remove_redundant_nops(b); - } - - /** Assembly **/ /* Order of basic blocks must have been determined by now */ if (normalize_jumps(g) < 0) { goto error; } + /* TODO: move this into optimize_cfg */ if (add_checks_for_loads_of_unknown_variables(g->g_entryblock, c) < 0) { goto error; } @@ -8695,6 +8698,8 @@ assemble(struct compiler *c, int addNone) goto error; } + + /* Create assembler */ if (!assemble_init(&a, c->u->u_firstlineno)) goto error; @@ -9464,7 +9469,7 @@ propagate_line_numbers(basicblock *entryblock) { /* Calculate the actual jump target from the target_label */ static int -calculate_jump_targets(basicblock *entryblock) +translate_jump_labels_to_targets(basicblock *entryblock) { int max_label = -1; for (basicblock *b = entryblock; b != NULL; b = b->b_next) { @@ -9783,7 +9788,7 @@ _PyCompile_OptimizeCfg(PyObject *instructions, PyObject *consts) if (const_cache == NULL) { goto error; } - if (calculate_jump_targets(g.g_entryblock)) { + if (translate_jump_labels_to_targets(g.g_entryblock)) { goto error; } if (optimize_cfg(&g, consts, const_cache) < 0) { From 8a2f28e10fef1fb041d5ac3e52cdff9dbb5ef7f7 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 29 Sep 2022 13:57:45 +0100 Subject: [PATCH 08/11] move check_loads function to optimization stage --- Python/compile.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 094810e5daba5a..211aa9047edc21 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8658,6 +8658,9 @@ assemble(struct compiler *c, int addNone) if (optimize_cfg(g, consts, c->c_const_cache)) { goto error; } + if (add_checks_for_loads_of_unknown_variables(g->g_entryblock, c) < 0) { + goto error; + } /** line numbers (TODO: move this to desugaring stage) */ if (duplicate_exits_without_lineno(g) < 0) { @@ -8684,11 +8687,6 @@ assemble(struct compiler *c, int addNone) goto error; } - /* TODO: move this into optimize_cfg */ - if (add_checks_for_loads_of_unknown_variables(g->g_entryblock, c) < 0) { - goto error; - } - assert(no_redundant_jumps(g)); /* Can't modify the bytecode after computing jump offsets. */ @@ -8698,8 +8696,6 @@ assemble(struct compiler *c, int addNone) goto error; } - - /* Create assembler */ if (!assemble_init(&a, c->u->u_firstlineno)) goto error; From 85751ea45c49539918a806bb33fc8d9d011b8ef5 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 3 Oct 2022 22:18:06 +0100 Subject: [PATCH 09/11] mark's review comments --- Python/compile.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 211aa9047edc21..5eaa664a560a20 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7975,8 +7975,8 @@ scan_block_for_local(int target, basicblock *b, bool unsafe_to_start, #undef MAYBE_PUSH static int -add_checks_for_loads_of_unknown_variables(basicblock *entryblock, - struct compiler *c) +add_checks_for_loads_of_uninitialized_variables(basicblock *entryblock, + struct compiler *c) { basicblock **stack = make_cfg_traversal_stack(entryblock); if (stack == NULL) { @@ -8639,7 +8639,7 @@ assemble(struct compiler *c, int addNone) } nlocalsplus -= numdropped; - /** Desugaring **/ + /** Map labels to targets and mark exception handlers **/ if (translate_jump_labels_to_targets(g->g_entryblock)) { goto error; } @@ -8658,7 +8658,7 @@ assemble(struct compiler *c, int addNone) if (optimize_cfg(g, consts, c->c_const_cache)) { goto error; } - if (add_checks_for_loads_of_unknown_variables(g->g_entryblock, c) < 0) { + if (add_checks_for_loads_of_uninitialized_variables(g->g_entryblock, c) < 0) { goto error; } @@ -8669,11 +8669,11 @@ assemble(struct compiler *c, int addNone) propagate_line_numbers(g->g_entryblock); guarantee_lineno_for_exits(g->g_entryblock, c->u->u_firstlineno); - /** Assembly **/ if (push_cold_blocks_to_end(g, code_flags) < 0) { goto error; } + /** Assembly **/ int maxdepth = stackdepth(g->g_entryblock, code_flags); if (maxdepth < 0) { goto error; From 5f78cc76cc12368fc959bb256c1aaa8602353d29 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 3 Oct 2022 22:19:58 +0100 Subject: [PATCH 10/11] fix comment --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 5eaa664a560a20..39e47a60d33406 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8662,7 +8662,7 @@ assemble(struct compiler *c, int addNone) goto error; } - /** line numbers (TODO: move this to desugaring stage) */ + /** line numbers (TODO: move this before optimization stage) */ if (duplicate_exits_without_lineno(g) < 0) { goto error; } From e44b087f515e6313857d58b833e10a69ea163dbe Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 4 Oct 2022 19:09:37 +0100 Subject: [PATCH 11/11] Call it preprocessing --- Python/compile.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index 39e47a60d33406..2da36d0f6316e0 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8639,7 +8639,8 @@ assemble(struct compiler *c, int addNone) } nlocalsplus -= numdropped; - /** Map labels to targets and mark exception handlers **/ + /** Preprocessing **/ + /* Map labels to targets and mark exception handlers */ if (translate_jump_labels_to_targets(g->g_entryblock)) { goto error; }