Skip to content

Commit 0315941

Browse files
authored
gh-114265: remove i_loc_propagated, jump threading does not consider line numbers anymore (#114535)
1 parent ea3cd04 commit 0315941

File tree

2 files changed

+49
-42
lines changed

2 files changed

+49
-42
lines changed

Lib/test/test_peepholer.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -1150,10 +1150,11 @@ def get_insts(lno1, lno2, op1, op2):
11501150
lno1, lno2 = (4, 5)
11511151
with self.subTest(lno = (lno1, lno2), ops = (op1, op2)):
11521152
insts = get_insts(lno1, lno2, op1, op2)
1153+
op = 'JUMP' if 'JUMP' in (op1, op2) else 'JUMP_NO_INTERRUPT'
11531154
expected_insts = [
11541155
('LOAD_NAME', 0, 10),
11551156
('NOP', 0, 4),
1156-
(op2, 0, 5),
1157+
(op, 0, 5),
11571158
]
11581159
self.cfg_optimization_test(insts, expected_insts, consts=list(range(5)))
11591160

Python/flowgraph.c

+47-41
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ typedef struct _PyCfgInstruction {
2929
int i_opcode;
3030
int i_oparg;
3131
_PyCompilerSrcLocation i_loc;
32-
unsigned i_loc_propagated : 1; /* location was set by propagate_line_numbers */
3332
struct _PyCfgBasicblock *i_target; /* target block (if jump instruction) */
3433
struct _PyCfgBasicblock *i_except; /* target block when exception is raised */
3534
} cfg_instr;
@@ -146,6 +145,16 @@ basicblock_next_instr(basicblock *b)
146145
return b->b_iused++;
147146
}
148147

148+
static cfg_instr *
149+
basicblock_last_instr(const basicblock *b) {
150+
assert(b->b_iused >= 0);
151+
if (b->b_iused > 0) {
152+
assert(b->b_instr != NULL);
153+
return &b->b_instr[b->b_iused - 1];
154+
}
155+
return NULL;
156+
}
157+
149158
/* Allocate a new block and return a pointer to it.
150159
Returns NULL on error.
151160
*/
@@ -186,6 +195,22 @@ basicblock_addop(basicblock *b, int opcode, int oparg, location loc)
186195
return SUCCESS;
187196
}
188197

198+
static int
199+
basicblock_add_jump(basicblock *b, int opcode, basicblock *target, location loc)
200+
{
201+
cfg_instr *last = basicblock_last_instr(b);
202+
if (last && is_jump(last)) {
203+
return ERROR;
204+
}
205+
206+
RETURN_IF_ERROR(
207+
basicblock_addop(b, opcode, target->b_label.id, loc));
208+
last = basicblock_last_instr(b);
209+
assert(last && last->i_opcode == opcode);
210+
last->i_target = target;
211+
return SUCCESS;
212+
}
213+
189214
static inline int
190215
basicblock_append_instructions(basicblock *target, basicblock *source)
191216
{
@@ -199,16 +224,6 @@ basicblock_append_instructions(basicblock *target, basicblock *source)
199224
return SUCCESS;
200225
}
201226

202-
static cfg_instr *
203-
basicblock_last_instr(const basicblock *b) {
204-
assert(b->b_iused >= 0);
205-
if (b->b_iused > 0) {
206-
assert(b->b_instr != NULL);
207-
return &b->b_instr[b->b_iused - 1];
208-
}
209-
return NULL;
210-
}
211-
212227
static inline int
213228
basicblock_nofallthrough(const basicblock *b) {
214229
cfg_instr *last = basicblock_last_instr(b);
@@ -560,8 +575,8 @@ normalize_jumps_in_block(cfg_builder *g, basicblock *b) {
560575
if (backwards_jump == NULL) {
561576
return ERROR;
562577
}
563-
basicblock_addop(backwards_jump, JUMP, target->b_label.id, last->i_loc);
564-
backwards_jump->b_instr[0].i_target = target;
578+
RETURN_IF_ERROR(
579+
basicblock_add_jump(backwards_jump, JUMP, target, last->i_loc));
565580
last->i_opcode = reversed_opcode;
566581
last->i_target = b->b_next;
567582

@@ -1141,13 +1156,7 @@ remove_redundant_jumps(cfg_builder *g) {
11411156
basicblock *next = next_nonempty_block(b->b_next);
11421157
if (jump_target == next) {
11431158
changes++;
1144-
if (last->i_loc_propagated) {
1145-
b->b_iused--;
1146-
}
1147-
else {
1148-
assert(last->i_loc.lineno != -1);
1149-
INSTR_SET_OP0(last, NOP);
1150-
}
1159+
INSTR_SET_OP0(last, NOP);
11511160
}
11521161
}
11531162
}
@@ -1184,23 +1193,23 @@ inline_small_exit_blocks(basicblock *bb) {
11841193
// target->i_target using the provided opcode. Return whether or not the
11851194
// optimization was successful.
11861195
static bool
1187-
jump_thread(cfg_instr *inst, cfg_instr *target, int opcode)
1196+
jump_thread(basicblock *bb, cfg_instr *inst, cfg_instr *target, int opcode)
11881197
{
11891198
assert(is_jump(inst));
11901199
assert(is_jump(target));
1200+
assert(inst == basicblock_last_instr(bb));
11911201
// bpo-45773: If inst->i_target == target->i_target, then nothing actually
11921202
// changes (and we fall into an infinite loop):
1193-
if (inst->i_loc.lineno == -1) assert(inst->i_loc_propagated);
1194-
if (target->i_loc.lineno == -1) assert(target->i_loc_propagated);
1195-
if ((inst->i_loc.lineno == target->i_loc.lineno ||
1196-
inst->i_loc_propagated || target->i_loc_propagated) &&
1197-
inst->i_target != target->i_target)
1198-
{
1199-
inst->i_target = target->i_target;
1200-
inst->i_opcode = opcode;
1201-
if (inst->i_loc_propagated && !target->i_loc_propagated) {
1202-
inst->i_loc = target->i_loc;
1203-
}
1203+
if (inst->i_target != target->i_target) {
1204+
/* Change inst to NOP and append a jump to target->i_target. The
1205+
* NOP will be removed later if it's not needed for the lineno.
1206+
*/
1207+
INSTR_SET_OP0(inst, NOP);
1208+
1209+
RETURN_IF_ERROR(
1210+
basicblock_add_jump(
1211+
bb, opcode, target->i_target, target->i_loc));
1212+
12041213
return true;
12051214
}
12061215
return false;
@@ -1673,29 +1682,29 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
16731682
case POP_JUMP_IF_NONE:
16741683
switch (target->i_opcode) {
16751684
case JUMP:
1676-
i -= jump_thread(inst, target, inst->i_opcode);
1685+
i -= jump_thread(bb, inst, target, inst->i_opcode);
16771686
}
16781687
break;
16791688
case POP_JUMP_IF_FALSE:
16801689
switch (target->i_opcode) {
16811690
case JUMP:
1682-
i -= jump_thread(inst, target, POP_JUMP_IF_FALSE);
1691+
i -= jump_thread(bb, inst, target, POP_JUMP_IF_FALSE);
16831692
}
16841693
break;
16851694
case POP_JUMP_IF_TRUE:
16861695
switch (target->i_opcode) {
16871696
case JUMP:
1688-
i -= jump_thread(inst, target, POP_JUMP_IF_TRUE);
1697+
i -= jump_thread(bb, inst, target, POP_JUMP_IF_TRUE);
16891698
}
16901699
break;
16911700
case JUMP:
16921701
case JUMP_NO_INTERRUPT:
16931702
switch (target->i_opcode) {
16941703
case JUMP:
1695-
i -= jump_thread(inst, target, JUMP);
1704+
i -= jump_thread(bb, inst, target, JUMP);
16961705
continue;
16971706
case JUMP_NO_INTERRUPT:
1698-
i -= jump_thread(inst, target, opcode);
1707+
i -= jump_thread(bb, inst, target, opcode);
16991708
continue;
17001709
}
17011710
break;
@@ -1707,7 +1716,7 @@ optimize_basic_block(PyObject *const_cache, basicblock *bb, PyObject *consts)
17071716
* of FOR_ITER.
17081717
*/
17091718
/*
1710-
i -= jump_thread(inst, target, FOR_ITER);
1719+
i -= jump_thread(bb, inst, target, FOR_ITER);
17111720
*/
17121721
}
17131722
break;
@@ -2410,7 +2419,6 @@ propagate_line_numbers(basicblock *entryblock) {
24102419
for (int i = 0; i < b->b_iused; i++) {
24112420
if (b->b_instr[i].i_loc.lineno < 0) {
24122421
b->b_instr[i].i_loc = prev_location;
2413-
b->b_instr[i].i_loc_propagated = 1;
24142422
}
24152423
else {
24162424
prev_location = b->b_instr[i].i_loc;
@@ -2420,7 +2428,6 @@ propagate_line_numbers(basicblock *entryblock) {
24202428
if (b->b_next->b_iused > 0) {
24212429
if (b->b_next->b_instr[0].i_loc.lineno < 0) {
24222430
b->b_next->b_instr[0].i_loc = prev_location;
2423-
b->b_next->b_instr[0].i_loc_propagated = 1;
24242431
}
24252432
}
24262433
}
@@ -2429,7 +2436,6 @@ propagate_line_numbers(basicblock *entryblock) {
24292436
if (target->b_predecessors == 1) {
24302437
if (target->b_instr[0].i_loc.lineno < 0) {
24312438
target->b_instr[0].i_loc = prev_location;
2432-
target->b_instr[0].i_loc_propagated = 1;
24332439
}
24342440
}
24352441
}

0 commit comments

Comments
 (0)