From 79a02686ba5dc3c4eee6064a7f2864debe6bfd32 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Thu, 25 Aug 2022 18:35:23 +0100 Subject: [PATCH 1/6] gh-93678: apply remove_redundant_jumps in optimize_cfg --- Lib/test/test_peepholer.py | 6 +++--- Python/compile.c | 44 ++++++++++++++++++++++++++++---------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index 7ece468363be58..7cd9b060f5e90b 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -892,7 +892,8 @@ def test_conditional_jump_forward_non_const_condition(self): self.cfg_optimization_test(insts, expected, consts=list(range(5))) def test_conditional_jump_forward_const_condition(self): - # The unreachable branch of the jump is removed + # The unreachable branch of the jump is removed, the jump + # becomes redundant and is replaced by a NOP (for the lineno) insts = [ ('LOAD_CONST', 3, 11), @@ -903,8 +904,7 @@ def test_conditional_jump_forward_const_condition(self): ] expected = [ ('NOP', None, 11), - ('JUMP', lbl := self.Label(), 12), - lbl, + ('NOP', None, 12), ('LOAD_CONST', '3', 14) ] self.cfg_optimization_test(insts, expected, consts=list(range(5))) diff --git a/Python/compile.c b/Python/compile.c index 627f86a8ce9188..874ce844a2cc62 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -7396,6 +7396,9 @@ mark_cold(basicblock *entryblock) { return 0; } +static int +remove_redundant_jumps(cfg_builder *g); + static int push_cold_blocks_to_end(cfg_builder *g, int code_flags) { basicblock *entryblock = g->g_entryblock; @@ -7465,6 +7468,12 @@ push_cold_blocks_to_end(cfg_builder *g, int code_flags) { } assert(b != NULL && b->b_next == NULL); b->b_next = cold_blocks; + + if (cold_blocks != NULL) { + if (remove_redundant_jumps(g) < 0) { + return -1; + } + } return 0; } @@ -8269,9 +8278,6 @@ trim_unused_consts(basicblock *entryblock, PyObject *consts); static int duplicate_exits_without_lineno(cfg_builder *g); -static int -extend_block(basicblock *bb); - static int * build_cellfixedoffsets(struct compiler *c) { @@ -8476,6 +8482,19 @@ propagate_line_numbers(basicblock *entryblock); static void eliminate_empty_basic_blocks(cfg_builder *g); +#ifdef Py_DEBUG +static void +assert_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) { + assert(last->i_target != b->b_next); + } + } + } +} +#endif static int remove_redundant_jumps(cfg_builder *g) { @@ -8592,8 +8611,8 @@ assemble(struct compiler *c, int addNone) if (trim_unused_consts(g->g_entryblock, consts)) { goto error; } - if (duplicate_exits_without_lineno(g)) { - return NULL; + 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); @@ -8612,10 +8631,6 @@ assemble(struct compiler *c, int addNone) if (push_cold_blocks_to_end(g, code_flags) < 0) { goto error; } - - if (remove_redundant_jumps(g) < 0) { - goto error; - } for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { clean_basic_block(b); } @@ -8627,6 +8642,10 @@ assemble(struct compiler *c, int addNone) goto error; } +#ifdef Py_DEBUG + assert_no_redundant_jumps(g); +#endif + /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(g->g_entryblock); @@ -9488,7 +9507,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache) } } for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - if (extend_block(b)) { + if (extend_block(b) < 0) { return -1; } } @@ -9500,7 +9519,7 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache) assert(b->b_predecessors == 0); } for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { - if (extend_block(b)) { + if (extend_block(b) < 0) { return -1; } } @@ -9517,6 +9536,9 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache) for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { clean_basic_block(b); } + if (remove_redundant_jumps(g) < 0) { + return -1; + } return 0; } From d0ffa1560b310e4021f2f16504880b163f414d93 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 30 Aug 2022 13:11:22 +0100 Subject: [PATCH 2/6] assert_no_redundant_jumps --> no_redundant_jumps --- Python/compile.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 874ce844a2cc62..d961ff95440225 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8483,16 +8483,18 @@ static void eliminate_empty_basic_blocks(cfg_builder *g); #ifdef Py_DEBUG -static void -assert_no_redundant_jumps(cfg_builder *g) { +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) { assert(last->i_target != b->b_next); + return false; } } } + return true; } #endif @@ -8642,9 +8644,7 @@ assemble(struct compiler *c, int addNone) goto error; } -#ifdef Py_DEBUG - assert_no_redundant_jumps(g); -#endif + assert(no_redundant_jumps(g)); /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(g->g_entryblock); From e47688ae7b83d174708051f96496a1154ff476e1 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 30 Aug 2022 23:26:48 +0100 Subject: [PATCH 3/6] ASSERT_NO_REDUNDANT_JUMPS macro --- Python/compile.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index d961ff95440225..034d5babba1a15 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8496,8 +8496,13 @@ no_redundant_jumps(cfg_builder *g) { } return true; } + +#define ASSERT_NO_REDUNDANT_JUMPS(G) assert(no_redundant_jumps(G)) +#else +#define ASSERT_NO_REDUNDANT_JUMPS(G) #endif + static int remove_redundant_jumps(cfg_builder *g) { /* If a non-empty block ends with a jump instruction, check if the next @@ -8644,7 +8649,7 @@ assemble(struct compiler *c, int addNone) goto error; } - assert(no_redundant_jumps(g)); + ASSERT_NO_REDUNDANT_JUMPS(g); /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(g->g_entryblock); From 6803fb5c715b88f03d7c706d60b6b00677161458 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 31 Aug 2022 23:48:20 +0100 Subject: [PATCH 4/6] Revert "ASSERT_NO_REDUNDANT_JUMPS macro" This reverts commit e47688ae7b83d174708051f96496a1154ff476e1. --- Python/compile.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Python/compile.c b/Python/compile.c index 034d5babba1a15..d961ff95440225 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8496,13 +8496,8 @@ no_redundant_jumps(cfg_builder *g) { } return true; } - -#define ASSERT_NO_REDUNDANT_JUMPS(G) assert(no_redundant_jumps(G)) -#else -#define ASSERT_NO_REDUNDANT_JUMPS(G) #endif - static int remove_redundant_jumps(cfg_builder *g) { /* If a non-empty block ends with a jump instruction, check if the next @@ -8649,7 +8644,7 @@ assemble(struct compiler *c, int addNone) goto error; } - ASSERT_NO_REDUNDANT_JUMPS(g); + assert(no_redundant_jumps(g)); /* Can't modify the bytecode after computing jump offsets. */ assemble_jump_offsets(g->g_entryblock); From 566d9c0309c2c6bac58740c7b4914e7f5d2f83ea Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 31 Aug 2022 23:50:15 +0100 Subject: [PATCH 5/6] Py_DEBUG --> NDEBUG --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index d961ff95440225..dc640411d9a2ba 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8482,7 +8482,7 @@ propagate_line_numbers(basicblock *entryblock); static void eliminate_empty_basic_blocks(cfg_builder *g); -#ifdef Py_DEBUG +#ifdef NDEBUG static bool no_redundant_jumps(cfg_builder *g) { for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) { From 9c06493d465a3e30be492413a8195a6e5ce125f0 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 31 Aug 2022 23:52:48 +0100 Subject: [PATCH 6/6] ifdef --> ifndef --- Python/compile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/compile.c b/Python/compile.c index dc640411d9a2ba..857fca440ac0a7 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -8482,7 +8482,7 @@ propagate_line_numbers(basicblock *entryblock); static void eliminate_empty_basic_blocks(cfg_builder *g); -#ifdef NDEBUG +#ifndef NDEBUG static bool no_redundant_jumps(cfg_builder *g) { for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {