gh-113603: Compiler no longer tries to maintain the no-empty-block invariant (#113636)

This commit is contained in:
Irit Katriel 2024-01-03 16:57:48 +00:00 committed by GitHub
parent 0c3455a969
commit 7d01fb4808
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 52 additions and 78 deletions

View File

@ -448,6 +448,19 @@ class TestSpecifics(unittest.TestCase):
# See gh-113054
compile('if (5 if 5 else T): 0', '<eval>', 'exec')
def test_condition_expression_with_redundant_comparisons_compiles(self):
# See gh-113054
compile('if 9<9<9and 9or 9:9', '<eval>', 'exec')
def test_dead_code_with_except_handler_compiles(self):
compile(textwrap.dedent("""
if None:
with CM:
x = 1
else:
x = 2
"""), '<eval>', 'exec')
def test_compile_invalid_namedexpr(self):
# gh-109351
m = ast.Module(

View File

@ -0,0 +1 @@
Fixed bug where a redundant NOP is not removed, causing an assertion to fail in the compiler in debug mode.

View File

@ -449,6 +449,15 @@ _PyCfgBuilder_Addop(cfg_builder *g, int opcode, int oparg, location loc)
}
static basicblock *
next_nonempty_block(basicblock *b)
{
while (b && b->b_iused == 0) {
b = b->b_next;
}
return b;
}
/***** debugging helpers *****/
#ifndef NDEBUG
@ -464,24 +473,16 @@ no_redundant_nops(cfg_builder *g) {
return true;
}
static bool
no_empty_basic_blocks(cfg_builder *g) {
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
if (b->b_iused == 0) {
return false;
}
}
return true;
}
static bool
no_redundant_jumps(cfg_builder *g) {
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
cfg_instr *last = basicblock_last_instr(b);
if (last != NULL) {
if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
assert(last->i_target != b->b_next);
if (last->i_target == b->b_next) {
basicblock *next = next_nonempty_block(b->b_next);
basicblock *jump_target = next_nonempty_block(last->i_target);
assert(jump_target != next);
if (jump_target == next) {
return false;
}
}
@ -961,42 +962,6 @@ mark_reachable(basicblock *entryblock) {
return SUCCESS;
}
static void
eliminate_empty_basic_blocks(cfg_builder *g) {
/* Eliminate empty blocks */
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
basicblock *next = b->b_next;
while (next && next->b_iused == 0) {
next = next->b_next;
}
b->b_next = next;
}
while(g->g_entryblock && g->g_entryblock->b_iused == 0) {
g->g_entryblock = g->g_entryblock->b_next;
}
int next_lbl = get_max_label(g->g_entryblock) + 1;
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
assert(b->b_iused > 0);
for (int i = 0; i < b->b_iused; i++) {
cfg_instr *instr = &b->b_instr[i];
if (HAS_TARGET(instr->i_opcode)) {
basicblock *target = instr->i_target;
while (target->b_iused == 0) {
target = target->b_next;
}
if (instr->i_target != target) {
if (!IS_LABEL(target->b_label)) {
target->b_label.id = next_lbl++;
}
instr->i_target = target;
instr->i_oparg = target->b_label.id;
}
assert(instr->i_target && instr->i_target->b_iused > 0);
}
}
}
}
static int
remove_redundant_nops(basicblock *bb) {
/* Remove NOPs when legal to do so. */
@ -1025,10 +990,7 @@ remove_redundant_nops(basicblock *bb) {
}
}
else {
basicblock* next = bb->b_next;
while (next && next->b_iused == 0) {
next = next->b_next;
}
basicblock *next = next_nonempty_block(bb->b_next);
/* or if last instruction in BB and next BB has same line number */
if (next) {
location next_loc = NO_LOCATION;
@ -1112,25 +1074,22 @@ remove_redundant_jumps(cfg_builder *g) {
* can be deleted.
*/
assert(no_empty_basic_blocks(g));
bool remove_empty_blocks = false;
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
cfg_instr *last = basicblock_last_instr(b);
assert(last != NULL);
if (last == NULL) {
continue;
}
assert(!IS_ASSEMBLER_OPCODE(last->i_opcode));
if (IS_UNCONDITIONAL_JUMP_OPCODE(last->i_opcode)) {
if (last->i_target == NULL) {
basicblock* jump_target = next_nonempty_block(last->i_target);
if (jump_target == NULL) {
PyErr_SetString(PyExc_SystemError, "jump with NULL target");
return ERROR;
}
if (last->i_target == b->b_next) {
assert(b->b_next->b_iused);
basicblock *next = next_nonempty_block(b->b_next);
if (jump_target == next) {
if (last->i_loc.lineno == NO_LOCATION.lineno) {
b->b_iused--;
if (b->b_iused == 0) {
remove_empty_blocks = true;
}
}
else {
INSTR_SET_OP0(last, NOP);
@ -1138,10 +1097,6 @@ remove_redundant_jumps(cfg_builder *g) {
}
}
}
if (remove_empty_blocks) {
eliminate_empty_basic_blocks(g);
}
assert(no_empty_basic_blocks(g));
return SUCCESS;
}
@ -1749,11 +1704,9 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
{
assert(PyDict_CheckExact(const_cache));
RETURN_IF_ERROR(check_cfg(g));
eliminate_empty_basic_blocks(g);
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
RETURN_IF_ERROR(inline_small_exit_blocks(b));
}
assert(no_empty_basic_blocks(g));
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);
@ -1768,14 +1721,21 @@ optimize_cfg(cfg_builder *g, PyObject *consts, PyObject *const_cache)
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);
}
eliminate_empty_basic_blocks(g);
assert(no_redundant_nops(g));
RETURN_IF_ERROR(remove_redundant_jumps(g));
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
remove_redundant_nops(b);
}
RETURN_IF_ERROR(remove_redundant_jumps(g));
assert(no_redundant_jumps(g));
return SUCCESS;
}
@ -1825,7 +1785,6 @@ insert_superinstructions(cfg_builder *g)
for (basicblock *b = g->g_entryblock; b != NULL; b = b->b_next) {
remove_redundant_nops(b);
}
eliminate_empty_basic_blocks(g);
assert(no_redundant_nops(g));
}
@ -2299,8 +2258,6 @@ is_exit_without_lineno(basicblock *b) {
static int
duplicate_exits_without_lineno(cfg_builder *g)
{
assert(no_empty_basic_blocks(g));
int next_lbl = get_max_label(g->g_entryblock) + 1;
/* Copy all exit blocks without line number that are targets of a jump.
@ -2308,9 +2265,11 @@ duplicate_exits_without_lineno(cfg_builder *g)
basicblock *entryblock = g->g_entryblock;
for (basicblock *b = entryblock; b != NULL; b = b->b_next) {
cfg_instr *last = basicblock_last_instr(b);
assert(last != NULL);
if (last == NULL) {
continue;
}
if (is_jump(last)) {
basicblock *target = last->i_target;
basicblock *target = next_nonempty_block(last->i_target);
if (is_exit_without_lineno(target) && target->b_predecessors > 1) {
basicblock *new_target = copy_basicblock(g, target);
if (new_target == NULL) {
@ -2367,9 +2326,10 @@ propagate_line_numbers(basicblock *entryblock) {
}
}
if (BB_HAS_FALLTHROUGH(b) && b->b_next->b_predecessors == 1) {
assert(b->b_next->b_iused);
if (b->b_next->b_instr[0].i_loc.lineno < 0) {
b->b_next->b_instr[0].i_loc = prev_location;
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;
}
}
}
if (is_jump(last)) {