Skip to content

GH-105229: Replace some superinstructions with single instruction equivalent. #105230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 18 additions & 18 deletions Include/internal/pycore_opcode.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 2 additions & 5 deletions Include/internal/pycore_opcode_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,8 @@ extern "C" {
(opcode) == RERAISE)

#define IS_SUPERINSTRUCTION_OPCODE(opcode) \
((opcode) == LOAD_FAST__LOAD_FAST || \
(opcode) == LOAD_FAST__LOAD_CONST || \
(opcode) == LOAD_CONST__LOAD_FAST || \
(opcode) == STORE_FAST__LOAD_FAST || \
(opcode) == STORE_FAST__STORE_FAST)
((opcode) == LOAD_FAST__LOAD_CONST || \
(opcode) == LOAD_CONST__LOAD_FAST)


#define LOG_BITS_PER_INT 5
Expand Down
28 changes: 14 additions & 14 deletions Include/opcode.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions Lib/dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@
LOAD_SUPER_ATTR = opmap['LOAD_SUPER_ATTR']
CALL_INTRINSIC_1 = opmap['CALL_INTRINSIC_1']
CALL_INTRINSIC_2 = opmap['CALL_INTRINSIC_2']
LOAD_FAST_LOAD_FAST = opmap['LOAD_FAST_LOAD_FAST']
STORE_FAST_LOAD_FAST = opmap['STORE_FAST_LOAD_FAST']
STORE_FAST_STORE_FAST = opmap['STORE_FAST_STORE_FAST']

CACHE = opmap["CACHE"]

Expand Down Expand Up @@ -493,6 +496,13 @@ def _get_instructions_bytes(code, varname_from_oparg=None,
argval = offset + 2 + signed_arg*2
argval += 2 * caches
argrepr = "to " + repr(argval)
elif deop in (LOAD_FAST_LOAD_FAST, STORE_FAST_LOAD_FAST, STORE_FAST_STORE_FAST):
arg1 = arg >> 4
arg2 = arg & 15
val1, argrepr1 = _get_name_info(arg1, varname_from_oparg)
val2, argrepr2 = _get_name_info(arg2, varname_from_oparg)
argrepr = argrepr1 + ", " + argrepr2
argval = val1, val2
elif deop in haslocal or deop in hasfree:
argval, argrepr = _get_name_info(arg, varname_from_oparg)
elif deop in hascompare:
Expand Down
3 changes: 2 additions & 1 deletion Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.12b1 3530 (Shrink the LOAD_SUPER_ATTR caches)
# Python 3.12b1 3531 (Add PEP 695 changes)
# Python 3.13a1 3550 (Plugin optimizer support)
# Python 3.13a1 3551 (Compact superinstructions)

# Python 3.14 will start with 3600

Expand All @@ -462,7 +463,7 @@ def _write_atomic(path, data, mode=0o666):
# Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
# in PC/launcher.c must also be updated.

MAGIC_NUMBER = (3550).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3551).to_bytes(2, 'little') + b'\r\n'

_RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c

Expand Down
8 changes: 3 additions & 5 deletions Lib/opcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,9 @@ def pseudo_op(name, op, real_ops):
def_op('DICT_MERGE', 164)
def_op('DICT_UPDATE', 165)

def_op('LOAD_FAST_LOAD_FAST', 168)
def_op('STORE_FAST_LOAD_FAST', 169)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, how are the stats on this? Is it worth keeping?

I would expect most of these pairs to span multiple lines. Although maybe comprehensions and generator expressions make up for it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, as you say it might be quite low.
I think we should keep it for this PR anyway, as we are replacing two-codeunit superinstructions with the one-codeunit equivalent. We can change the choice of superinstructions in another PR.

def_op('STORE_FAST_STORE_FAST', 170)
def_op('CALL', 171)
def_op('KW_NAMES', 172)
hasconst.append(172)
Expand Down Expand Up @@ -411,7 +414,6 @@ def pseudo_op(name, op, real_ops):
],
"LOAD_FAST": [
"LOAD_FAST__LOAD_CONST",
"LOAD_FAST__LOAD_FAST",
],
"LOAD_GLOBAL": [
"LOAD_GLOBAL_BUILTIN",
Expand All @@ -422,10 +424,6 @@ def pseudo_op(name, op, real_ops):
"STORE_ATTR_SLOT",
"STORE_ATTR_WITH_HINT",
],
"STORE_FAST": [
"STORE_FAST__LOAD_FAST",
"STORE_FAST__STORE_FAST",
],
"STORE_SUBSCR": [
"STORE_SUBSCR_DICT",
"STORE_SUBSCR_LIST_INT",
Expand Down
49 changes: 23 additions & 26 deletions Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -760,15 +760,12 @@ def load_test(x, y=0):
dis_load_test_quickened_code = """\
%3d 0 RESUME 0

%3d 2 LOAD_FAST__LOAD_FAST 0 (x)
4 LOAD_FAST 1 (y)
6 STORE_FAST__STORE_FAST 3 (b)
8 STORE_FAST__LOAD_FAST 2 (a)

%3d 10 LOAD_FAST__LOAD_FAST 2 (a)
12 LOAD_FAST 3 (b)
14 BUILD_TUPLE 2
16 RETURN_VALUE
%3d 2 LOAD_FAST_LOAD_FAST 1 (x, y)
4 STORE_FAST_STORE_FAST 50 (b, a)

%3d 6 LOAD_FAST_LOAD_FAST 35 (a, b)
8 BUILD_TUPLE 2
10 RETURN_VALUE
""" % (load_test.__code__.co_firstlineno,
load_test.__code__.co_firstlineno + 1,
load_test.__code__.co_firstlineno + 2)
Expand Down Expand Up @@ -811,9 +808,8 @@ def extended_arg_quick():
%3d 2 LOAD_CONST 1 (Ellipsis)
4 EXTENDED_ARG 1
6 UNPACK_EX 256
8 STORE_FAST 0 (_)
10 STORE_FAST 0 (_)
12 RETURN_CONST 0 (None)
8 STORE_FAST_STORE_FAST 0 (_, _)
10 RETURN_CONST 0 (None)
"""% (extended_arg_quick.__code__.co_firstlineno,
extended_arg_quick.__code__.co_firstlineno + 1,)

Expand Down Expand Up @@ -1026,26 +1022,28 @@ def expected(count, w):
s = ['''\
1 %*d RESUME 0

''' % (w, 0)]
2 %*d LOAD_FAST 0 (x)
%*d LOAD_CONST 1 (1)
%*d BINARY_OP 0 (+)
''' % (w, 0, w, 2, w, 4, w, 6)]
s += ['''\
%*d LOAD_FAST 0 (x)
%*d STORE_FAST_LOAD_FAST 0 (x, x)
%*d LOAD_CONST 1 (1)
%*d BINARY_OP 0 (+)
%*d STORE_FAST 0 (x)
''' % (w, 10*i + 2, w, 10*i + 4, w, 10*i + 6, w, 10*i + 10)
for i in range(count)]
''' % (w, 8*i + 10, w, 8*i + 12, w, 8*i + 14)
for i in range(count-1)]
s += ['''\
%*d STORE_FAST 0 (x)

3 %*d LOAD_FAST 0 (x)
%*d RETURN_VALUE
''' % (w, 10*count + 2, w, 10*count + 4)]
s[1] = ' 2' + s[1][3:]
''' % (w, 8*count + 2, w, 8*count + 4, w, 8*count + 6)]
return ''.join(s)

for i in range(1, 5):
self.do_disassembly_test(func(i), expected(i, 4), True)
self.do_disassembly_test(func(999), expected(999, 4), True)
self.do_disassembly_test(func(1000), expected(1000, 5), True)
self.do_disassembly_test(func(1200), expected(1200, 4), True)
self.do_disassembly_test(func(1300), expected(1300, 5), True)

def test_disassemble_str(self):
self.do_disassembly_test(expr_str, dis_expr_str)
Expand Down Expand Up @@ -1646,11 +1644,10 @@ def _prepare_test_cases():
Instruction(opname='LOAD_DEREF', opcode=137, arg=3, argval='b', argrepr='b', offset=16, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_DEREF', opcode=137, arg=4, argval='c', argrepr='c', offset=18, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_DEREF', opcode=137, arg=5, argval='d', argrepr='d', offset=20, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=0, argval='e', argrepr='e', offset=22, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=1, argval='f', argrepr='f', offset=24, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=6, argval=6, argrepr='', offset=26, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=34, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_CONST', opcode=121, arg=0, argval=None, argrepr='None', offset=36, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST_LOAD_FAST', opcode=168, arg=1, argval=('e', 'f'), argrepr='e, f', offset=22, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='CALL', opcode=171, arg=6, argval=6, argrepr='', offset=24, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='POP_TOP', opcode=1, arg=None, argval=None, argrepr='', offset=32, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RETURN_CONST', opcode=121, arg=0, argval=None, argrepr='None', offset=34, starts_line=None, is_jump_target=False, positions=None),
]


Expand Down
8 changes: 4 additions & 4 deletions Lib/test/test_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -1158,8 +1158,8 @@ def func():
('line', 'func', 5),
('line', 'meth', 1),
('jump', 'func', 5, 5),
('jump', 'func', 5, '[offset=114]'),
('branch', 'func', '[offset=120]', '[offset=122]'),
('jump', 'func', 5, '[offset=112]'),
('branch', 'func', '[offset=118]', '[offset=120]'),
('line', 'check_events', 11)])

self.check_events(func, recorders = FLOW_AND_LINE_RECORDERS, expected = [
Expand All @@ -1174,8 +1174,8 @@ def func():
('line', 'meth', 1),
('return', None),
('jump', 'func', 5, 5),
('jump', 'func', 5, '[offset=114]'),
('branch', 'func', '[offset=120]', '[offset=122]'),
('jump', 'func', 5, '[offset=112]'),
('branch', 'func', '[offset=118]', '[offset=120]'),
('return', None),
('line', 'check_events', 11)])

Expand Down
18 changes: 15 additions & 3 deletions Lib/test/test_peepholer.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ def test_load_fast_known_simple(self):
def f():
x = 1
y = x + x
self.assertInBytecode(f, 'LOAD_FAST')
self.assertInBytecode(f, 'LOAD_FAST_LOAD_FAST')

def test_load_fast_unknown_simple(self):
def f():
Expand Down Expand Up @@ -790,7 +790,10 @@ def f():
print(a00, a01, a62, a63)
print(a64, a65, a78, a79)

for i in 0, 1, 62, 63:
self.assertInBytecode(f, 'LOAD_FAST_LOAD_FAST', ("a00", "a01"))
self.assertNotInBytecode(f, 'LOAD_FAST_CHECK', "a00")
self.assertNotInBytecode(f, 'LOAD_FAST_CHECK', "a01")
for i in 62, 63:
# First 64 locals: analyze completely
self.assertInBytecode(f, 'LOAD_FAST', f"a{i:02}")
self.assertNotInBytecode(f, 'LOAD_FAST_CHECK', f"a{i:02}")
Expand Down Expand Up @@ -1071,7 +1074,16 @@ def test_no_unsafe_static_swap(self):
('POP_TOP', 0, 4),
('RETURN_VALUE', 5)
]
self.cfg_optimization_test(insts, insts, consts=list(range(3)), nlocals=1)
expected_insts = [
('LOAD_CONST', 0, 1),
('LOAD_CONST', 1, 2),
('LOAD_CONST', 2, 3),
('SWAP', 3, 4),
('STORE_FAST_STORE_FAST', 17, 4),
('POP_TOP', 0, 4),
('RETURN_VALUE', 5)
]
self.cfg_optimization_test(insts, expected_insts, consts=list(range(3)), nlocals=1)

if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Replace some dynamic superinstructions with single instruction equivalents.
Loading