gh-114265: remove i_loc_propagated, jump threading does not consider line numbers anymore (#114535)

This commit is contained in:
Irit Katriel 2024-01-25 12:54:19 +00:00 committed by GitHub
parent ea3cd0498c
commit 0315941441
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 49 additions and 42 deletions

View File

@ -1150,10 +1150,11 @@ class DirectCfgOptimizerTests(CfgOptimizationTestCase):
lno1, lno2 = (4, 5) lno1, lno2 = (4, 5)
with self.subTest(lno = (lno1, lno2), ops = (op1, op2)): with self.subTest(lno = (lno1, lno2), ops = (op1, op2)):
insts = get_insts(lno1, lno2, op1, op2) insts = get_insts(lno1, lno2, op1, op2)
op = 'JUMP' if 'JUMP' in (op1, op2) else 'JUMP_NO_INTERRUPT'
expected_insts = [ expected_insts = [
('LOAD_NAME', 0, 10), ('LOAD_NAME', 0, 10),
('NOP', 0, 4), ('NOP', 0, 4),
(op2, 0, 5), (op, 0, 5),
] ]
self.cfg_optimization_test(insts, expected_insts, consts=list(range(5))) self.cfg_optimization_test(insts, expected_insts, consts=list(range(5)))

View File

@ -29,7 +29,6 @@ typedef struct _PyCfgInstruction {
int i_opcode; int i_opcode;
int i_oparg; int i_oparg;
_PyCompilerSrcLocation i_loc; _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_target; /* target block (if jump instruction) */
struct _PyCfgBasicblock *i_except; /* target block when exception is raised */ struct _PyCfgBasicblock *i_except; /* target block when exception is raised */
} cfg_instr; } cfg_instr;
@ -146,6 +145,16 @@ basicblock_next_instr(basicblock *b)
return b->b_iused++; return b->b_iused++;
} }
static cfg_instr *
basicblock_last_instr(const basicblock *b) {
assert(b->b_iused >= 0);
if (b->b_iused > 0) {
assert(b->b_instr != NULL);
return &b->b_instr[b->b_iused - 1];
}
return NULL;
}
/* Allocate a new block and return a pointer to it. /* Allocate a new block and return a pointer to it.
Returns NULL on error. Returns NULL on error.
*/ */
@ -186,6 +195,22 @@ basicblock_addop(basicblock *b, int opcode, int oparg, location loc)
return SUCCESS; return SUCCESS;
} }
static int
basicblock_add_jump(basicblock *b, int opcode, basicblock *target, location loc)
{
cfg_instr *last = basicblock_last_instr(b);
if (last && is_jump(last)) {
return ERROR;
}
RETURN_IF_ERROR(
basicblock_addop(b, opcode, target->b_label.id, loc));
last = basicblock_last_instr(b);
assert(last && last->i_opcode == opcode);
last->i_target = target;
return SUCCESS;
}
static inline int static inline int
basicblock_append_instructions(basicblock *target, basicblock *source) basicblock_append_instructions(basicblock *target, basicblock *source)
{ {
@ -199,16 +224,6 @@ basicblock_append_instructions(basicblock *target, basicblock *source)
return SUCCESS; return SUCCESS;
} }
static cfg_instr *
basicblock_last_instr(const basicblock *b) {
assert(b->b_iused >= 0);
if (b->b_iused > 0) {
assert(b->b_instr != NULL);
return &b->b_instr[b->b_iused - 1];
}
return NULL;
}
static inline int static inline int
basicblock_nofallthrough(const basicblock *b) { basicblock_nofallthrough(const basicblock *b) {
cfg_instr *last = basicblock_last_instr(b); cfg_instr *last = basicblock_last_instr(b);
@ -560,8 +575,8 @@ normalize_jumps_in_block(cfg_builder *g, basicblock *b) {
if (backwards_jump == NULL) { if (backwards_jump == NULL) {
return ERROR; return ERROR;
} }
basicblock_addop(backwards_jump, JUMP, target->b_label.id, last->i_loc); RETURN_IF_ERROR(
backwards_jump->b_instr[0].i_target = target; basicblock_add_jump(backwards_jump, JUMP, target, last->i_loc));
last->i_opcode = reversed_opcode; last->i_opcode = reversed_opcode;
last->i_target = b->b_next; last->i_target = b->b_next;
@ -1141,13 +1156,7 @@ remove_redundant_jumps(cfg_builder *g) {
basicblock *next = next_nonempty_block(b->b_next); basicblock *next = next_nonempty_block(b->b_next);
if (jump_target == next) { if (jump_target == next) {
changes++; changes++;
if (last->i_loc_propagated) { INSTR_SET_OP0(last, NOP);
b->b_iused--;
}
else {
assert(last->i_loc.lineno != -1);
INSTR_SET_OP0(last, NOP);
}
} }
} }
} }
@ -1184,23 +1193,23 @@ inline_small_exit_blocks(basicblock *bb) {
// target->i_target using the provided opcode. Return whether or not the // target->i_target using the provided opcode. Return whether or not the
// optimization was successful. // optimization was successful.
static bool static bool
jump_thread(cfg_instr *inst, cfg_instr *target, int opcode) jump_thread(basicblock *bb, cfg_instr *inst, cfg_instr *target, int opcode)
{ {
assert(is_jump(inst)); assert(is_jump(inst));
assert(is_jump(target)); assert(is_jump(target));
assert(inst == basicblock_last_instr(bb));
// bpo-45773: If inst->i_target == target->i_target, then nothing actually // bpo-45773: If inst->i_target == target->i_target, then nothing actually
// changes (and we fall into an infinite loop): // changes (and we fall into an infinite loop):
if (inst->i_loc.lineno == -1) assert(inst->i_loc_propagated); if (inst->i_target != target->i_target) {
if (target->i_loc.lineno == -1) assert(target->i_loc_propagated); /* Change inst to NOP and append a jump to target->i_target. The
if ((inst->i_loc.lineno == target->i_loc.lineno || * NOP will be removed later if it's not needed for the lineno.
inst->i_loc_propagated || target->i_loc_propagated) && */
inst->i_target != target->i_target) INSTR_SET_OP0(inst, NOP);
{
inst->i_target = target->i_target; RETURN_IF_ERROR(
inst->i_opcode = opcode; basicblock_add_jump(
if (inst->i_loc_propagated && !target->i_loc_propagated) { bb, opcode, target->i_target, target->i_loc));
inst->i_loc = target->i_loc;
}
return true; return true;
} }
return false; return false;
@ -1673,29 +1682,29 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
case POP_JUMP_IF_NONE: case POP_JUMP_IF_NONE:
switch (target->i_opcode) { switch (target->i_opcode) {
case JUMP: case JUMP:
i -= jump_thread(inst, target, inst->i_opcode); i -= jump_thread(bb, inst, target, inst->i_opcode);
} }
break; break;
case POP_JUMP_IF_FALSE: case POP_JUMP_IF_FALSE:
switch (target->i_opcode) { switch (target->i_opcode) {
case JUMP: case JUMP:
i -= jump_thread(inst, target, POP_JUMP_IF_FALSE); i -= jump_thread(bb, inst, target, POP_JUMP_IF_FALSE);
} }
break; break;
case POP_JUMP_IF_TRUE: case POP_JUMP_IF_TRUE:
switch (target->i_opcode) { switch (target->i_opcode) {
case JUMP: case JUMP:
i -= jump_thread(inst, target, POP_JUMP_IF_TRUE); i -= jump_thread(bb, inst, target, POP_JUMP_IF_TRUE);
} }
break; break;
case JUMP: case JUMP:
case JUMP_NO_INTERRUPT: case JUMP_NO_INTERRUPT:
switch (target->i_opcode) { switch (target->i_opcode) {
case JUMP: case JUMP:
i -= jump_thread(inst, target, JUMP); i -= jump_thread(bb, inst, target, JUMP);
continue; continue;
case JUMP_NO_INTERRUPT: case JUMP_NO_INTERRUPT:
i -= jump_thread(inst, target, opcode); i -= jump_thread(bb, inst, target, opcode);
continue; continue;
} }
break; break;
@ -1707,7 +1716,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
* of FOR_ITER. * of FOR_ITER.
*/ */
/* /*
i -= jump_thread(inst, target, FOR_ITER); i -= jump_thread(bb, inst, target, FOR_ITER);
*/ */
} }
break; break;
@ -2410,7 +2419,6 @@ propagate_line_numbers(basicblock *entryblock) {
for (int i = 0; i < b->b_iused; i++) { for (int i = 0; i < b->b_iused; i++) {
if (b->b_instr[i].i_loc.lineno < 0) { if (b->b_instr[i].i_loc.lineno < 0) {
b->b_instr[i].i_loc = prev_location; b->b_instr[i].i_loc = prev_location;
b->b_instr[i].i_loc_propagated = 1;
} }
else { else {
prev_location = b->b_instr[i].i_loc; prev_location = b->b_instr[i].i_loc;
@ -2420,7 +2428,6 @@ propagate_line_numbers(basicblock *entryblock) {
if (b->b_next->b_iused > 0) { if (b->b_next->b_iused > 0) {
if (b->b_next->b_instr[0].i_loc.lineno < 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 = prev_location;
b->b_next->b_instr[0].i_loc_propagated = 1;
} }
} }
} }
@ -2429,7 +2436,6 @@ propagate_line_numbers(basicblock *entryblock) {
if (target->b_predecessors == 1) { if (target->b_predecessors == 1) {
if (target->b_instr[0].i_loc.lineno < 0) { if (target->b_instr[0].i_loc.lineno < 0) {
target->b_instr[0].i_loc = prev_location; target->b_instr[0].i_loc = prev_location;
target->b_instr[0].i_loc_propagated = 1;
} }
} }
} }