Skip to content

bpo-47120: define virtual opcodes in opcode.py. Better organization of jump opcodes. #32353

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

Closed
wants to merge 3 commits into from
Closed
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
92 changes: 57 additions & 35 deletions Include/opcode.h

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
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.11a6 3490 (remove JUMP_IF_NOT_EXC_MATCH, add CHECK_EXC_MATCH)
# Python 3.11a6 3491 (remove JUMP_IF_NOT_EG_MATCH, add CHECK_EG_MATCH,
# add JUMP_BACKWARD_NO_INTERRUPT, make JUMP_NO_INTERRUPT virtual)
# Python 3.11a7 3492 (Reorganize jump opcodes)

# Python 3.12 will start with magic number 3500

Expand All @@ -414,7 +415,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 = (3491).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3492).to_bytes(2, 'little') + b'\r\n'
_RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c

_PYCACHE = '__pycache__'
Expand Down
74 changes: 53 additions & 21 deletions Lib/opcode.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,47 @@
hascompare = []
hasfree = []
hasnargs = [] # unused
assembler = []

opmap = {}
opname = ['<%r>' % (op,) for op in range(256)]
virtual = {}

RELATIVE = 0x1
ABSOLUTE = 0x2
Copy link
Member

Choose a reason for hiding this comment

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

Aren't we about to remove all absolute jumps?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and then this flag will not be needed.

BACKWARD = 0x4
FORWARD = 0x8
ASSEMBLER = 0x10 # Not emitted in codegen stage, only by assembler

_inline_cache_entries = [0] * 256


def jump_flags(op, flags):
assert flags & RELATIVE != flags & ABSOLUTE
if flags & RELATIVE:
assert not (flags & BACKWARD) or not (flags & FORWARD)
hasjrel.append(op)
elif flags & ABSOLUTE:
hasjabs.append(op)
if flags & ASSEMBLER:
assembler.append(op)

def def_op(name, op, entries=0):
opname[op] = name
opmap[name] = op
if op >= 0:
opname[op] = name
opmap[name] = op
else:
virtual[name] = op
_inline_cache_entries[op] = entries

def jump_op(flags, name, op, entries=0):
jump_flags(op, flags)
def_op(name, op, entries=entries)

def name_op(name, op, entries=0):
def_op(name, op, entries)
hasname.append(op)

def jrel_op(name, op, entries=0):
def_op(name, op, entries)
hasjrel.append(op)

def jabs_op(name, op, entries=0):
def_op(name, op, entries)
hasjabs.append(op)

# Instruction opcodes for compiled code
# Blank lines correspond to available opcodes
Expand Down Expand Up @@ -111,7 +130,6 @@ def jabs_op(name, op, entries=0):
name_op('STORE_NAME', 90) # Index in name list
name_op('DELETE_NAME', 91) # ""
def_op('UNPACK_SEQUENCE', 92, 1) # Number of tuple items
jrel_op('FOR_ITER', 93)
def_op('UNPACK_EX', 94)
name_op('STORE_ATTR', 95, 4) # Index in name list
name_op('DELETE_ATTR', 96) # ""
Expand All @@ -130,31 +148,22 @@ def jabs_op(name, op, entries=0):
hascompare.append(107)
name_op('IMPORT_NAME', 108) # Index in name list
name_op('IMPORT_FROM', 109) # Index in name list
jrel_op('JUMP_FORWARD', 110) # Number of words to skip
jabs_op('JUMP_IF_FALSE_OR_POP', 111) # Target byte offset from beginning of code
jabs_op('JUMP_IF_TRUE_OR_POP', 112) # ""
jabs_op('POP_JUMP_IF_FALSE', 114) # ""
jabs_op('POP_JUMP_IF_TRUE', 115) # ""
name_op('LOAD_GLOBAL', 116, 5) # Index in name list
def_op('IS_OP', 117)
def_op('CONTAINS_OP', 118)
def_op('RERAISE', 119)
def_op('COPY', 120)
def_op('BINARY_OP', 122, 1)
jrel_op('SEND', 123) # Number of bytes to skip
def_op('LOAD_FAST', 124) # Local variable number
haslocal.append(124)
def_op('STORE_FAST', 125) # Local variable number
haslocal.append(125)
def_op('DELETE_FAST', 126) # Local variable number
haslocal.append(126)
jabs_op('POP_JUMP_IF_NOT_NONE', 128)
jabs_op('POP_JUMP_IF_NONE', 129)
def_op('RAISE_VARARGS', 130) # Number of raise arguments (1, 2, or 3)
def_op('GET_AWAITABLE', 131)
def_op('MAKE_FUNCTION', 132) # Flags
def_op('BUILD_SLICE', 133) # Number of items
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)
Expand All @@ -165,7 +174,6 @@ def jabs_op(name, op, entries=0):
hasfree.append(138)
def_op('DELETE_DEREF', 139)
hasfree.append(139)
jrel_op('JUMP_BACKWARD', 140) # Number of words to skip (backwards)

def_op('CALL_FUNCTION_EX', 142) # Flags

Expand Down Expand Up @@ -197,8 +205,32 @@ def jabs_op(name, op, entries=0):
def_op('KW_NAMES', 172)
hasconst.append(172)

jump_op(RELATIVE | FORWARD, 'FOR_ITER', 173)
jump_op(RELATIVE | FORWARD, 'SEND', 174)
jump_op(RELATIVE | FORWARD | ASSEMBLER, 'JUMP_FORWARD', 175)
jump_op(RELATIVE | BACKWARD | ASSEMBLER, 'JUMP_BACKWARD', 176)
jump_op(RELATIVE | BACKWARD | ASSEMBLER, 'JUMP_BACKWARD_NO_INTERRUPT', 177)

jump_op(ABSOLUTE, 'JUMP_IF_FALSE_OR_POP', 178)
jump_op(ABSOLUTE, 'JUMP_IF_TRUE_OR_POP', 179)
jump_op(ABSOLUTE, 'POP_JUMP_IF_FALSE', 180)
jump_op(ABSOLUTE, 'POP_JUMP_IF_TRUE', 181)
jump_op(ABSOLUTE, 'POP_JUMP_IF_NOT_NONE', 182)
jump_op(ABSOLUTE, 'POP_JUMP_IF_NONE', 183)


# Virtual Opcodes
# (Pseudo-instructions used in the compiler,
# but turned into NOPs by the assembler)

def_op('SETUP_FINALLY', -1)
Copy link
Member

Choose a reason for hiding this comment

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

These are an internal detail of the compiler. They never exist in the final bytecode, so I don't think they should be here.

Copy link
Member Author

Choose a reason for hiding this comment

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

The point was to autogenerate some of the macros around them.

Copy link
Member

Choose a reason for hiding this comment

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

If the code to generate the macro is large than macros, that suggests to me that it isn't worth it.

Copy link
Member Author

@iritkatriel iritkatriel Apr 6, 2022

Choose a reason for hiding this comment

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

For me it's not about how long the code is, but about how many places we need to change when we add/remove/change an opcode (i.e., maintainability).

def_op('SETUP_CLEANUP', -2)
def_op('SETUP_WITH', -3)
def_op('POP_BLOCK', -4)
jump_op(RELATIVE, 'JUMP', -5)
jump_op(RELATIVE, 'JUMP_NO_INTERRUPT', -6)

del def_op, name_op, jrel_op, jabs_op
del def_op, name_op, jump_op, jump_flags

_nb_ops = [
("NB_ADD", "+"),
Expand Down
Loading