Skip to content

Commit 4af1265

Browse files
gh-159493: insert super instructions after borrow analysis in compiler (GH-160032)
1 parent 85599d3 commit 4af1265

4 files changed

Lines changed: 87 additions & 45 deletions

File tree

‎Lib/test/test_dis.py‎

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,26 +1817,25 @@ def _prepare_test_cases():
18171817
make_inst(opname='MAKE_CELL', arg=1, argval='b', argrepr='b', offset=2, start_offset=2, starts_line=False, line_number=None),
18181818
make_inst(opname='RESUME', arg=0, argval=0, argrepr='', offset=4, start_offset=4, starts_line=True, line_number=1, cache_info=[('counter', 1, b'\x00\x00')]),
18191819
make_inst(opname='LOAD_CONST', arg=3, argval=(3, 4), argrepr='(3, 4)', offset=8, start_offset=8, starts_line=True, line_number=2),
1820-
make_inst(opname='LOAD_FAST_BORROW', arg=0, argval='a', argrepr='a', offset=10, start_offset=10, starts_line=False, line_number=2),
1821-
make_inst(opname='LOAD_FAST_BORROW', arg=1, argval='b', argrepr='b', offset=12, start_offset=12, starts_line=False, line_number=2),
1822-
make_inst(opname='BUILD_TUPLE', arg=2, argval=2, argrepr='', offset=14, start_offset=14, starts_line=False, line_number=2),
1823-
make_inst(opname='LOAD_CONST', arg=1, argval=code_object_f, argrepr=repr(code_object_f), offset=16, start_offset=16, starts_line=False, line_number=2),
1824-
make_inst(opname='MAKE_FUNCTION', arg=None, argval=None, argrepr='', offset=18, start_offset=18, starts_line=False, line_number=2),
1825-
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=8, argval=8, argrepr='closure', offset=20, start_offset=20, starts_line=False, line_number=2),
1826-
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=1, argval=1, argrepr='defaults', offset=22, start_offset=22, starts_line=False, line_number=2),
1827-
make_inst(opname='STORE_FAST', arg=2, argval='f', argrepr='f', offset=24, start_offset=24, starts_line=False, line_number=2),
1828-
make_inst(opname='LOAD_GLOBAL', arg=1, argval='print', argrepr='print + NULL', offset=26, start_offset=26, starts_line=True, line_number=7, cache_info=[('counter', 1, b'\x00\x00'), ('index', 1, b'\x00\x00'), ('module_keys_version', 1, b'\x00\x00'), ('builtin_keys_version', 1, b'\x00\x00')]),
1829-
make_inst(opname='LOAD_DEREF', arg=0, argval='a', argrepr='a', offset=36, start_offset=36, starts_line=False, line_number=7),
1830-
make_inst(opname='LOAD_DEREF', arg=1, argval='b', argrepr='b', offset=38, start_offset=38, starts_line=False, line_number=7),
1831-
make_inst(opname='LOAD_COMMON_CONSTANT', arg=8, argval='', argrepr="''", offset=40, start_offset=40, starts_line=False, line_number=7),
1832-
make_inst(opname='LOAD_SMALL_INT', arg=1, argval=1, argrepr='', offset=42, start_offset=42, starts_line=False, line_number=7),
1833-
make_inst(opname='BUILD_LIST', arg=0, argval=0, argrepr='', offset=44, start_offset=44, starts_line=False, line_number=7),
1834-
make_inst(opname='BUILD_MAP', arg=0, argval=0, argrepr='', offset=46, start_offset=46, starts_line=False, line_number=7),
1835-
make_inst(opname='LOAD_CONST', arg=2, argval='Hello world!', argrepr="'Hello world!'", offset=48, start_offset=48, starts_line=False, line_number=7),
1836-
make_inst(opname='CALL', arg=7, argval=7, argrepr='', offset=50, start_offset=50, starts_line=False, line_number=7, cache_info=[('counter', 1, b'\x00\x00'), ('func_version', 2, b'\x00\x00\x00\x00')]),
1837-
make_inst(opname='POP_TOP', arg=None, argval=None, argrepr='', offset=58, start_offset=58, starts_line=False, line_number=7),
1838-
make_inst(opname='LOAD_FAST_BORROW', arg=2, argval='f', argrepr='f', offset=60, start_offset=60, starts_line=True, line_number=8),
1839-
make_inst(opname='RETURN_VALUE', arg=None, argval=None, argrepr='', offset=62, start_offset=62, starts_line=False, line_number=8),
1820+
make_inst(opname='LOAD_FAST_BORROW_LOAD_FAST_BORROW', arg=1, argval=('a', 'b'), argrepr='a, b', offset=10, start_offset=10, starts_line=False, line_number=2),
1821+
make_inst(opname='BUILD_TUPLE', arg=2, argval=2, argrepr='', offset=12, start_offset=12, starts_line=False, line_number=2),
1822+
make_inst(opname='LOAD_CONST', arg=1, argval=code_object_f, argrepr=repr(code_object_f), offset=14, start_offset=14, starts_line=False, line_number=2),
1823+
make_inst(opname='MAKE_FUNCTION', arg=None, argval=None, argrepr='', offset=16, start_offset=16, starts_line=False, line_number=2),
1824+
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=8, argval=8, argrepr='closure', offset=18, start_offset=18, starts_line=False, line_number=2),
1825+
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=1, argval=1, argrepr='defaults', offset=20, start_offset=20, starts_line=False, line_number=2),
1826+
make_inst(opname='STORE_FAST', arg=2, argval='f', argrepr='f', offset=22, start_offset=22, starts_line=False, line_number=2),
1827+
make_inst(opname='LOAD_GLOBAL', arg=1, argval='print', argrepr='print + NULL', offset=24, start_offset=24, starts_line=True, line_number=7, cache_info=[('counter', 1, b'\x00\x00'), ('index', 1, b'\x00\x00'), ('module_keys_version', 1, b'\x00\x00'), ('builtin_keys_version', 1, b'\x00\x00')]),
1828+
make_inst(opname='LOAD_DEREF', arg=0, argval='a', argrepr='a', offset=34, start_offset=34, starts_line=False, line_number=7),
1829+
make_inst(opname='LOAD_DEREF', arg=1, argval='b', argrepr='b', offset=36, start_offset=36, starts_line=False, line_number=7),
1830+
make_inst(opname='LOAD_COMMON_CONSTANT', arg=8, argval='', argrepr="''", offset=38, start_offset=38, starts_line=False, line_number=7),
1831+
make_inst(opname='LOAD_SMALL_INT', arg=1, argval=1, argrepr='', offset=40, start_offset=40, starts_line=False, line_number=7),
1832+
make_inst(opname='BUILD_LIST', arg=0, argval=0, argrepr='', offset=42, start_offset=42, starts_line=False, line_number=7),
1833+
make_inst(opname='BUILD_MAP', arg=0, argval=0, argrepr='', offset=44, start_offset=44, starts_line=False, line_number=7),
1834+
make_inst(opname='LOAD_CONST', arg=2, argval='Hello world!', argrepr="'Hello world!'", offset=46, start_offset=46, starts_line=False, line_number=7),
1835+
make_inst(opname='CALL', arg=7, argval=7, argrepr='', offset=48, start_offset=48, starts_line=False, line_number=7, cache_info=[('counter', 1, b'\x00\x00'), ('func_version', 2, b'\x00\x00\x00\x00')]),
1836+
make_inst(opname='POP_TOP', arg=None, argval=None, argrepr='', offset=56, start_offset=56, starts_line=False, line_number=7),
1837+
make_inst(opname='LOAD_FAST_BORROW', arg=2, argval='f', argrepr='f', offset=58, start_offset=58, starts_line=True, line_number=8),
1838+
make_inst(opname='RETURN_VALUE', arg=None, argval=None, argrepr='', offset=60, start_offset=60, starts_line=False, line_number=8),
18401839
]
18411840

18421841
expected_opinfo_f = [
@@ -1845,25 +1844,23 @@ def _prepare_test_cases():
18451844
make_inst(opname='MAKE_CELL', arg=1, argval='d', argrepr='d', offset=4, start_offset=4, starts_line=False, line_number=None),
18461845
make_inst(opname='RESUME', arg=0, argval=0, argrepr='', offset=6, start_offset=6, starts_line=True, line_number=2, cache_info=[('counter', 1, b'\x00\x00')]),
18471846
make_inst(opname='LOAD_CONST', arg=2, argval=(5, 6), argrepr='(5, 6)', offset=10, start_offset=10, starts_line=True, line_number=3),
1848-
make_inst(opname='LOAD_FAST_BORROW', arg=3, argval='a', argrepr='a', offset=12, start_offset=12, starts_line=False, line_number=3),
1849-
make_inst(opname='LOAD_FAST_BORROW', arg=4, argval='b', argrepr='b', offset=14, start_offset=14, starts_line=False, line_number=3),
1850-
make_inst(opname='LOAD_FAST_BORROW', arg=0, argval='c', argrepr='c', offset=16, start_offset=16, starts_line=False, line_number=3),
1851-
make_inst(opname='LOAD_FAST_BORROW', arg=1, argval='d', argrepr='d', offset=18, start_offset=18, starts_line=False, line_number=3),
1852-
make_inst(opname='BUILD_TUPLE', arg=4, argval=4, argrepr='', offset=20, start_offset=20, starts_line=False, line_number=3),
1853-
make_inst(opname='LOAD_CONST', arg=1, argval=code_object_inner, argrepr=repr(code_object_inner), offset=22, start_offset=22, starts_line=False, line_number=3),
1854-
make_inst(opname='MAKE_FUNCTION', arg=None, argval=None, argrepr='', offset=24, start_offset=24, starts_line=False, line_number=3),
1855-
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=8, argval=8, argrepr='closure', offset=26, start_offset=26, starts_line=False, line_number=3),
1856-
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=1, argval=1, argrepr='defaults', offset=28, start_offset=28, starts_line=False, line_number=3),
1857-
make_inst(opname='STORE_FAST', arg=2, argval='inner', argrepr='inner', offset=30, start_offset=30, starts_line=False, line_number=3),
1858-
make_inst(opname='LOAD_GLOBAL', arg=1, argval='print', argrepr='print + NULL', offset=32, start_offset=32, starts_line=True, line_number=5, cache_info=[('counter', 1, b'\x00\x00'), ('index', 1, b'\x00\x00'), ('module_keys_version', 1, b'\x00\x00'), ('builtin_keys_version', 1, b'\x00\x00')]),
1859-
make_inst(opname='LOAD_DEREF', arg=3, argval='a', argrepr='a', offset=42, start_offset=42, starts_line=False, line_number=5),
1860-
make_inst(opname='LOAD_DEREF', arg=4, argval='b', argrepr='b', offset=44, start_offset=44, starts_line=False, line_number=5),
1861-
make_inst(opname='LOAD_DEREF', arg=0, argval='c', argrepr='c', offset=46, start_offset=46, starts_line=False, line_number=5),
1862-
make_inst(opname='LOAD_DEREF', arg=1, argval='d', argrepr='d', offset=48, start_offset=48, starts_line=False, line_number=5),
1863-
make_inst(opname='CALL', arg=4, argval=4, argrepr='', offset=50, start_offset=50, starts_line=False, line_number=5, cache_info=[('counter', 1, b'\x00\x00'), ('func_version', 2, b'\x00\x00\x00\x00')]),
1864-
make_inst(opname='POP_TOP', arg=None, argval=None, argrepr='', offset=58, start_offset=58, starts_line=False, line_number=5),
1865-
make_inst(opname='LOAD_FAST_BORROW', arg=2, argval='inner', argrepr='inner', offset=60, start_offset=60, starts_line=True, line_number=6),
1866-
make_inst(opname='RETURN_VALUE', arg=None, argval=None, argrepr='', offset=62, start_offset=62, starts_line=False, line_number=6),
1847+
make_inst(opname='LOAD_FAST_BORROW_LOAD_FAST_BORROW', arg=52, argval=('a', 'b'), argrepr='a, b', offset=12, start_offset=12, starts_line=False, line_number=3),
1848+
make_inst(opname='LOAD_FAST_BORROW_LOAD_FAST_BORROW', arg=1, argval=('c', 'd'), argrepr='c, d', offset=14, start_offset=14, starts_line=False, line_number=3),
1849+
make_inst(opname='BUILD_TUPLE', arg=4, argval=4, argrepr='', offset=16, start_offset=16, starts_line=False, line_number=3),
1850+
make_inst(opname='LOAD_CONST', arg=1, argval=code_object_inner, argrepr=repr(code_object_inner), offset=18, start_offset=18, starts_line=False, line_number=3),
1851+
make_inst(opname='MAKE_FUNCTION', arg=None, argval=None, argrepr='', offset=20, start_offset=20, starts_line=False, line_number=3),
1852+
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=8, argval=8, argrepr='closure', offset=22, start_offset=22, starts_line=False, line_number=3),
1853+
make_inst(opname='SET_FUNCTION_ATTRIBUTE', arg=1, argval=1, argrepr='defaults', offset=24, start_offset=24, starts_line=False, line_number=3),
1854+
make_inst(opname='STORE_FAST', arg=2, argval='inner', argrepr='inner', offset=26, start_offset=26, starts_line=False, line_number=3),
1855+
make_inst(opname='LOAD_GLOBAL', arg=1, argval='print', argrepr='print + NULL', offset=28, start_offset=28, starts_line=True, line_number=5, cache_info=[('counter', 1, b'\x00\x00'), ('index', 1, b'\x00\x00'), ('module_keys_version', 1, b'\x00\x00'), ('builtin_keys_version', 1, b'\x00\x00')]),
1856+
make_inst(opname='LOAD_DEREF', arg=3, argval='a', argrepr='a', offset=38, start_offset=38, starts_line=False, line_number=5),
1857+
make_inst(opname='LOAD_DEREF', arg=4, argval='b', argrepr='b', offset=40, start_offset=40, starts_line=False, line_number=5),
1858+
make_inst(opname='LOAD_DEREF', arg=0, argval='c', argrepr='c', offset=42, start_offset=42, starts_line=False, line_number=5),
1859+
make_inst(opname='LOAD_DEREF', arg=1, argval='d', argrepr='d', offset=44, start_offset=44, starts_line=False, line_number=5),
1860+
make_inst(opname='CALL', arg=4, argval=4, argrepr='', offset=46, start_offset=46, starts_line=False, line_number=5, cache_info=[('counter', 1, b'\x00\x00'), ('func_version', 2, b'\x00\x00\x00\x00')]),
1861+
make_inst(opname='POP_TOP', arg=None, argval=None, argrepr='', offset=54, start_offset=54, starts_line=False, line_number=5),
1862+
make_inst(opname='LOAD_FAST_BORROW', arg=2, argval='inner', argrepr='inner', offset=56, start_offset=56, starts_line=True, line_number=6),
1863+
make_inst(opname='RETURN_VALUE', arg=None, argval=None, argrepr='', offset=58, start_offset=58, starts_line=False, line_number=6),
18671864
]
18681865

18691866
expected_opinfo_inner = [

‎Lib/test/test_peepholer.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,31 @@ def f():
855855
y = x + x
856856
self.assertInBytecode(f, 'LOAD_FAST_BORROW_LOAD_FAST_BORROW')
857857

858+
def test_store_fast_load_fast_borrow(self):
859+
# gh-159493: a LOAD_FAST that can borrow must not be fused into
860+
# STORE_FAST_LOAD_FAST, regardless of how the source is laid out.
861+
def f(shared, n):
862+
for i in range(n): shared
863+
864+
def g(shared, n):
865+
for i in range(n):
866+
shared
867+
868+
for func in (f, g):
869+
with self.subTest(func=func.__name__):
870+
self.assertNotInBytecode(func, 'STORE_FAST_LOAD_FAST')
871+
self.assertInBytecode(func, 'LOAD_FAST_BORROW', 'shared')
872+
self.assertEqual([i.opname for i in dis.get_instructions(f)],
873+
[i.opname for i in dis.get_instructions(g)])
874+
875+
def test_store_fast_load_fast_not_borrowable(self):
876+
def f(x):
877+
y = x; z = y
878+
return z
879+
# The load of y is stored into z, so it needs a strong reference and
880+
# can still be fused.
881+
self.assertInBytecode(f, 'STORE_FAST_LOAD_FAST', ('y', 'y'))
882+
858883
def test_load_fast_unknown_simple(self):
859884
def f():
860885
if condition():
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Superinstructions are now formed after the compiler decides which
2+
:opcode:`LOAD_FAST` instructions can borrow their reference.
3+
This fixes reference count contention slowdown on the free-threaded build for single-line loops and comprehensions.

‎Python/flowgraph.c‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2719,6 +2719,12 @@ insert_superinstructions(cfg_builder *g)
27192719
make_super_instruction(inst, &b->b_instr[i + 1], LOAD_FAST_LOAD_FAST);
27202720
}
27212721
break;
2722+
case LOAD_FAST_BORROW:
2723+
if (nextop == LOAD_FAST_BORROW) {
2724+
make_super_instruction(inst, &b->b_instr[i + 1],
2725+
LOAD_FAST_BORROW_LOAD_FAST_BORROW);
2726+
}
2727+
break;
27222728
case STORE_FAST:
27232729
switch (nextop) {
27242730
case LOAD_FAST:
@@ -2732,9 +2738,9 @@ insert_superinstructions(cfg_builder *g)
27322738
}
27332739
}
27342740
}
2735-
int res = remove_redundant_nops(g);
2741+
RETURN_IF_ERROR(remove_redundant_nops(g));
27362742
assert(no_redundant_nops(g));
2737-
return res;
2743+
return SUCCESS;
27382744
}
27392745

27402746
#define NOT_LOCAL -1
@@ -3834,8 +3840,6 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache,
38343840
RETURN_IF_ERROR(
38353841
add_checks_for_loads_of_uninitialized_variables(
38363842
g->g_entryblock, nlocals, nparams));
3837-
RETURN_IF_ERROR(insert_superinstructions(g));
3838-
38393843
RETURN_IF_ERROR(push_cold_blocks_to_end(g));
38403844
RETURN_IF_ERROR(resolve_line_numbers(g, firstlineno));
38413845
// temporarily remove assert. See https://github.com/python/cpython/issues/125845
@@ -4157,11 +4161,19 @@ _PyCfg_OptimizedCfgToInstructionSequence(cfg_builder *g,
41574161
RETURN_IF_ERROR(normalize_jumps(g));
41584162
assert(no_redundant_jumps(g));
41594163

4160-
/* Can't modify the bytecode after inserting instructions that produce
4161-
* borrowed references.
4164+
/* After this point the bytecode may only be modified in ways that
4165+
* preserve the borrowed references computed here.
4166+
*
4167+
* NOTE: keep the passes below in sync with _PyCompile_OptimizeCfg().
41624168
*/
41634169
RETURN_IF_ERROR(optimize_load_fast(g));
41644170

4171+
/* Superinstructions are inserted after optimize_load_fast() so that a
4172+
* LOAD_FAST which can be borrowed is converted to LOAD_FAST_BORROW
4173+
* instead of being fused into a STORE_FAST_LOAD_FAST. Fusing adjacent
4174+
* instructions does not change which references are borrowed. */
4175+
RETURN_IF_ERROR(insert_superinstructions(g));
4176+
41654177
/* Can't modify the bytecode after computing jump offsets. */
41664178
if (_PyCfg_ToInstructionSequence(g, seq) < 0) {
41674179
return ERROR;
@@ -4254,10 +4266,15 @@ _PyCompile_OptimizeCfg(PyObject *seq, PyObject *consts, int nlocals)
42544266
goto error;
42554267
}
42564268

4269+
/* NOTE: keep in sync with _PyCfg_OptimizedCfgToInstructionSequence(). */
42574270
if (optimize_load_fast(g) != SUCCESS) {
42584271
goto error;
42594272
}
42604273

4274+
if (insert_superinstructions(g) < 0) {
4275+
goto error;
4276+
}
4277+
42614278
res = cfg_to_instruction_sequence(g);
42624279
error:
42634280
Py_DECREF(const_cache);

0 commit comments

Comments
 (0)