8000 gh-105775: Convert LOAD_CLOSURE to a pseudo-op by polynomialherder · Pull Request #106059 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-105775: Convert LOAD_CLOSURE to a pseudo-op #106059

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.

Sign up for GitHub

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 14 commits into from
Jun 29, 2023
Merged

gh-105775: Convert LOAD_CLOSURE to a pseudo-op #106059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
21d086e
gh-105775: Demote LOAD_CLOSURE to a pseudo-op
polynomialherder Jun 24, 2023
6921930
gh-106044: Fix whitespace
polynomialherder Jun 24, 2023
b111d10
gh-105775: Regenerate generated_cases.c.h
polynomialherder Jun 25, 2023
bbdbb99
Merge branch 'main' into Remove-LOAD-CLOSURE-105775
polynomialherder Jun 25, 2023
93b3138
gh-105775: Add unit test, correct NEWS/docs
polynomialherder Jun 27, 2023
32c1a70
Merge branch 'main' into Remove-LOAD-CLOSURE-105775
polynomialherder Jun 27, 2023
4103b9a
Merge branch 'Remove-LOAD-CLOSURE-105775' of github.com:polynomialher…
polynomialherder Jun 27, 2023
269933e
Merge branch 'main' into Remove-LOAD-CLOSURE-105775
polynomialherder Jun 27, 2023
a069593
Merge branch 'main' into Remove-LOAD-CLOSURE-105775
polynomialherder Jun 27, 2023
988189d
gh-105775: Update documentation, whitespace tweaks
polynomialherder Jun 27, 2023
968601d
Merge branch 'main' into Remove-LOAD-CLOSURE-105775
polynomialherder Jun 27, 2023
46fa9d5
Update news item
polynomialherder Jun 28, 2023
2b677b6
Merge branch 'main' into Remove-LOAD-CLOSURE-105775
polynomialherder Jun 28, 2023
ab8ce45
Merge branch 'main' into Remove-LOAD-CLOSURE-105775
polynomialherder Jun 28, 2023
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
23 changes: 11 additions & 12 deletions Doc/library/dis.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1291,18 +1291,6 @@ iterations of the loop.
.. versionadded:: 3.11


.. opcode:: LOAD_CLOSURE (i)

Pushes a reference to the cell contained in slot ``i`` of the "fast locals"
storage. The name of the variable is ``co_fastlocalnames[i]``.

Note that ``LOAD_CLOSURE`` is effectively an alias for ``LOAD_FAST``.
It exists to keep bytecode a little more readable.

.. versionchanged:: 3.11
``i`` is no longer offset by the length of ``co_varnames``.


.. opcode:: LOAD_DEREF (i)

Loads the cell contained in slot ``i`` of the "fast locals" storage.
Expand Down Expand Up @@ -1725,6 +1713,17 @@ but are replaced by real opcodes or removed before bytecode is generated.
Undirected relative jump instructions which are replaced by their
directed (forward/backward) counterparts by the assembler.

.. opcode:: LOAD_CLOSURE (i)

Pushes a reference to the cell contained in slot ``i`` of the "fast locals"
storage.

Note that ``LOAD_CLOSURE`` is replaced with ``LOAD_FAST`` in the assembler.

.. versionchanged:: 3.13
This opcode is now a pseudo-instruction.


.. opcode:: LOAD_METHOD

Optimized unbound method lookup. Emitted as a ``LOAD_ATTR`` opcode
Expand Down
15 changes: 8 additions & 7 deletions Include/internal/pycore_opcode.h
View file
Open in desktop

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

22 changes: 11 additions & 11 deletions Include/opcode.h
View file
Open in desktop

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

3 changes: 2 additions & 1 deletion Lib/importlib/_bootstrap_external.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.13a1 3553 (Add SET_FUNCTION_ATTRIBUTE)
# Python 3.13a1 3554 (more efficient bytecodes for f-strings)
# Python 3.13a1 3555 (generate specialized opcodes metadata from bytecodes.c)
# Python 3.13a1 3556 (Convert LOAD_CLOSURE to a pseudo-op)

# Python 3.14 will start with 3600

Expand All @@ -467,7 +468,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 = (3555).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3556).to_bytes(2, 'little') + b'\r\n'

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

Expand Down
3 changes: 1 addition & 2 deletions Lib/opcode.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,6 @@ def pseudo_op(name, op, real_ops):
jrel_op('JUMP_BACKWARD_NO_INTERRUPT', 134) # Number of words to skip (backwards)
def_op('MAKE_CELL', 135)
hasfree.append(135)
def_op('LOAD_CLOSURE', 136)
hasfree.append(136)
def_op('LOAD_DEREF', 137)
hasfree.append(137)
def_op('STORE_DEREF', 138)
Expand Down Expand Up @@ -293,6 +291,7 @@ def pseudo_op(name, op, real_ops):
pseudo_op('LOAD_ZERO_SUPER_ATTR', 265, ['LOAD_SUPER_ATTR'])

pseudo_op('STORE_FAST_MAYBE_NULL', 266, ['STORE_FAST'])
pseudo_op('LOAD_CLOSURE', 267, ['LOAD_FAST'])

MAX_PSEUDO_OPCODE = MIN_PSEUDO_OPCODE + len(_pseudo_ops) - 1

Expand Down
38 changes: 38 additions & 0 deletions Lib/test/test_compiler_assemble.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,41 @@ def test_simple_expr(self):
]
expected = {(3, 4) : 3.5, (-100, 200) : 50, (10, 18) : 14}
self.assemble_test(insts, metadata, expected)


def test_expression_with_pseudo_instruction_load_closure(self):

def mod_two(x):
def inner():
return x
return inner() % 2

inner_code = mod_two.__code__.co_consts[1]
assert isinstance(inner_code, types.CodeType)

metadata = {
'filename' : 'mod_two.py',
'name' : 'mod_two',
'qualname' : 'nested.mod_two',
'cellvars' : {'x' : 0},
'consts': {None: 0, inner_code: 1, 2: 2},
'argcount' : 1,
'varnames' : {'x' : 0},
}

instructions = [
('RESUME', 0,),
('PUSH_NULL', 0, 1),
('LOAD_CLOSURE', 0, 1),
('BUILD_TUPLE', 1, 1),
('LOAD_CONST', 1, 1),
('MAKE_FUNCTION', 0, 2),
('SET_FUNCTION_ATTRIBUTE', 8, 2),
('CALL', 0, 2), # (lambda: x)()
('LOAD_CONST', 2, 2), # 2
('BINARY_OP', 6, 2), # %
('RETURN_VALUE', 0, 2)
]

expected = {(0,): 0, (1,): 1, (2,): 0, (120,): 0, (121,): 1}
self.assemble_test(instructions, metadata, expected)
16 changes: 8 additions & 8 deletions Lib/test/test_dis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ def foo(x):

%3d RESUME 0

%3d LOAD_CLOSURE 0 (y)
%3d LOAD_FAST 0 (y)
BUILD_TUPLE 1
LOAD_CONST 1 (<code object foo at 0x..., file "%s", line %d>)
MAKE_FUNCTION
Expand All @@ -709,7 +709,7 @@ def foo(x):
%3d RESUME 0

%3d LOAD_GLOBAL 1 (NULL + list)
LOAD_CLOSURE 0 (x)
LOAD_FAST 0 (x)
BUILD_TUPLE 1
LOAD_CONST 1 (<code object <genexpr> at 0x..., file "%s", line %d>)
MAKE_FUNCTION
Expand Down Expand Up @@ -1596,8 +1596,8 @@ def _prepare_test_cases():
Instruction(opname='MAKE_CELL', opcode=135, arg=1, argval='b', argrepr='b', offset=2, start_offset=2, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RESUME', opcode=151, arg=0, argval=0, argrepr='', offset=4, start_offset=4, starts_line=1, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=5, argval=(3, 4), argrepr='(3, 4)', offset=6, start_offset=6, starts_line=2, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CLOSURE', opcode=136, arg=0, argval='a', argrepr='a', offset=8, start_offset=8, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CLOSURE', opcode=136, arg=1, argval='b', argrepr='b', offset=10, start_offset=10, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=0, argval='a', argrepr='a', offset=8, start_offset=8, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=1, argval='b', argrepr='b', offset=10, start_offset=10, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='BUILD_TUPLE', opcode=102, arg=2, argval=2, argrepr='', offset=12, start_offset=12, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=1, argval=code_object_f, argrepr=repr(code_object_f), offset=14, start_offset=14, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='MAKE_FUNCTION', opcode=24, arg=None, argval=None, argrepr='', offset=16, start_offset=16, starts_line=None, is_jump_target=False, positions=None),
Expand All @@ -1624,10 +1624,10 @@ def _prepare_test_cases():
Instruction(opname='MAKE_CELL', opcode=135, arg=1, argval='d', argrepr='d', offset=4, start_offset=4, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='RESUME', opcode=151, arg=0, argval=0, argrepr='', offset=6, start_offset=6, starts_line=2, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=2, argval=(5, 6), argrepr='(5, 6)', offset=8, start_offset=8, starts_line=3, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CLOSURE', opcode=136, arg=3, argval='a', argrepr='a', offset=10, start_offset=10, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CLOSURE', opcode=136, arg=4, argval='b', argrepr='b', offset=12, start_offset=12, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CLOSURE', opcode=136, arg=0, argval='c', argrepr='c', offset=14, start_offset=14, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CLOSURE', opcode=136, arg=1, argval='d', argrepr='d', offset=16, start_offset=16, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=3, argval='a', argrepr='a', offset=10, start_offset=10, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=4, argval='b', argrepr='b', offset=12, start_offset=12, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=0, argval='c', argrepr='c', offset=14, start_offset=14, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_FAST', opcode=124, arg=1, argval='d', argrepr='d', offset=16, start_offset=16, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='BUILD_TUPLE', opcode=102, arg=4, argval=4, argrepr='', offset=18, start_offset=18, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='LOAD_CONST', opcode=100, arg=1, argval=code_object_inner, argrepr=repr(code_object_inner), offset=20, start_offset=20, starts_line=None, is_jump_target=False, positions=None),
Instruction(opname='MAKE_FUNCTION', opcode=24, arg=None, argval=None, argrepr='', offset=22, start_offset=22, starts_line=None, is_jump_target=False, positions=None),
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2023-06-24-10-34-27.gh-issue-105775.OqjoGV.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
:opcode:`LOAD_CLOSURE` is now a pseudo-op.
1 change: 1 addition & 0 deletions PC/launcher.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1270,6 +1270,7 @@ static PYC_MAGIC magic_values[] = {
/* Allow 50 magic numbers per version from here on */
{ 3450, 3499, L"3.11" },
{ 3500, 3549, L"3.12" },
{ 3550, 3599, L"3.13" },
{ 0 }
};

Expand Down
9 changes: 3 additions & 6 deletions Python/bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,9 @@ dummy_func(
}
}

inst(LOAD_CLOSURE, (-- value)) {
/* We keep LOAD_CLOSURE so that the bytecode stays more readable. */
value = GETLOCAL(oparg);
ERROR_IF(value == NULL, unbound_local_error);
Copy link
Member

Choose a reason for hiding this comment

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

I note that we are implicitly eliminating this NULL check by converting LOAD_CLOSURE to LOAD_FAST. I think that this is actually just fine, because ensuring this local exists (and is a cell variable) is the responsibility of the compiler, not the code author. The compiler will always insert a MAKE_CELL at the top of the function for any cell variable, so I can't see any way this null check would ever have failed. If it ever did, that would indicate a compiler bug, so it's not clear that UnboundLocalError would ever have made sense here anyway.

Copy link
Member

Choose a reason for hiding this comment

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

@markshannon What do you think? We could replace it with LOAD_CLOSURE_CHECK instead to preserve this. There might be a way for a debugger to delete the variable.

Copy link
Member
@carljm carljm Jun 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There might be a way for a debugger to delete the variable.

I don't believe there is, unless the debugger is doing something unsupported and poking directly at the fastlocals array on a _PyInterpreterFrame. The supported way for debuggers to change the value of variables is via frame.f_locals dict, which is synced back to fast locals by PyFrame_LocalsToFast, and will only update cell contents, never overwrite or delete the cell itself.

Py_INCREF(value);
}
pseudo(LOAD_CLOSURE) = {
LOAD_FAST,
};

inst(LOAD_FAST_CHECK, (-- value)) {
value = GETLOCAL(oparg);
Expand Down
2 changes: 2 additions & 0 deletions Python/compile.c
View file
Open in desktop
797D
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,8 @@ stack_effect(int opcode, int oparg, int jump)

case STORE_FAST_MAYBE_NULL:
return -1;
case LOAD_CLOSURE:
return 1;
case LOAD_METHOD:
return 1;
case LOAD_SUPER_METHOD:
Expand Down
Loading
0