Skip to content

Commit b996b81

Browse files
committed
gh-152600: Remove LOAD_FAST/LOAD_FAST_BORROW; POP_TOP pairs
Changes - moved `add_checks_for_loads_of_uninitialized_variables` earlier so that a `LOAD_FAST` that should be a `LOAD_FAST_CHECK` are not removed. - refactored `remove_redundant_nops_and_pairs` to be more concise. Also extended it so that any `COPY` gets removed. I don't think there is any reason to only remove `COPY` with an `oparg` of 1. I don't think a NEWS entry is needed.
1 parent bc3fa17 commit b996b81

2 files changed

Lines changed: 42 additions & 29 deletions

File tree

Lib/test/test_peepholer.py

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2521,24 +2521,14 @@ def test_unoptimized_if_unconsumed(self):
25212521
insts = [
25222522
("LOAD_FAST", 0, 1),
25232523
("LOAD_FAST", 1, 2),
2524-
("POP_TOP", None, 3),
2524+
("LOAD_SMALL_INT", 1, 3),
2525+
("BINARY_OP", 0, 3),
25252526
]
25262527
expected = [
25272528
("LOAD_FAST", 0, 1),
25282529
("LOAD_FAST_BORROW", 1, 2),
2529-
("POP_TOP", None, 3),
2530-
]
2531-
self.check(insts, expected)
2532-
2533-
insts = [
2534-
("LOAD_FAST", 0, 1),
2535-
("COPY", 1, 2),
2536-
("POP_TOP", None, 3),
2537-
]
2538-
expected = [
2539-
("LOAD_FAST", 0, 1),
2540-
("NOP", None, 2),
2541-
("NOP", None, 3),
2530+
("LOAD_SMALL_INT", 1, 3),
2531+
("BINARY_OP", 0, 3),
25422532
]
25432533
self.check(insts, expected)
25442534

@@ -2862,6 +2852,33 @@ def f():
28622852
return var
28632853
self.assertEqual(f(), "1")
28642854

2855+
def test_load_fast_removed(self):
2856+
insts = [
2857+
("LOAD_FAST", 0, 1),
2858+
("LOAD_FAST", 1, 2),
2859+
("POP_TOP", None, 3),
2860+
]
2861+
expected = [
2862+
("LOAD_FAST", 0, 1),
2863+
("NOP", None, 2),
2864+
("NOP", None, 3),
2865+
]
2866+
self.check(insts, expected)
2867+
2868+
def test_copy(self):
2869+
for i in range(1, 5):
2870+
insts = [
2871+
("LOAD_FAST", 0, 1),
2872+
("COPY", i, 2),
2873+
("POP_TOP", None, 3),
2874+
]
2875+
expected = [
2876+
("LOAD_FAST", 0, 1),
2877+
("NOP", None, 2),
2878+
("NOP", None, 3),
2879+
]
2880+
self.check(insts, expected)
2881+
28652882

28662883

28672884
if __name__ == "__main__":

Python/flowgraph.c

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,21 +1147,16 @@ remove_redundant_nops_and_pairs(basicblock *entryblock)
11471147
prev_instr = instr;
11481148
instr = &b->b_instr[i];
11491149
int prev_opcode = prev_instr ? prev_instr->i_opcode : 0;
1150-
int prev_oparg = prev_instr ? prev_instr->i_oparg : 0;
11511150
int opcode = instr->i_opcode;
1152-
bool is_redundant_pair = false;
11531151
if (opcode == POP_TOP) {
1154-
if (loads_const(prev_opcode)) {
1155-
is_redundant_pair = true;
1152+
if (loads_const(prev_opcode)
1153+
|| prev_opcode == COPY
1154+
|| prev_opcode == LOAD_FAST)
1155+
{
1156+
INSTR_SET_OP0(prev_instr, NOP);
1157+
INSTR_SET_OP0(instr, NOP);
1158+
done = false;
11561159
}
1157-
else if (prev_opcode == COPY && prev_oparg == 1) {
1158-
is_redundant_pair = true;
1159-
}
1160-
}
1161-
if (is_redundant_pair) {
1162-
INSTR_SET_OP0(prev_instr, NOP);
1163-
INSTR_SET_OP0(instr, NOP);
1164-
done = false;
11651160
}
11661161
}
11671162
if ((instr && is_jump(instr)) || !BB_HAS_FALLTHROUGH(b)) {
@@ -3790,16 +3785,17 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache,
37903785
}
37913786
}
37923787

3788+
RETURN_IF_ERROR(
3789+
add_checks_for_loads_of_uninitialized_variables(
3790+
g->g_entryblock, nlocals, nparams));
3791+
37933792
int ret = optimize_cfg(g, consts, const_cache, consts_index, firstlineno);
37943793

37953794
_Py_hashtable_destroy(consts_index);
37963795

37973796
RETURN_IF_ERROR(ret);
37983797

37993798
RETURN_IF_ERROR(remove_unused_consts(g->g_entryblock, consts));
3800-
RETURN_IF_ERROR(
3801-
add_checks_for_loads_of_uninitialized_variables(
3802-
g->g_entryblock, nlocals, nparams));
38033799
RETURN_IF_ERROR(insert_superinstructions(g));
38043800

38053801
RETURN_IF_ERROR(push_cold_blocks_to_end(g));

0 commit comments

Comments
 (0)