Skip to content

Commit 6facc8d

Browse files
committed
Make less of a special case of _PUSH_FRAME
Instead, the special case is an opcode using SAVE_FRAME_STATE(). Introducing #if TIER_ONE and #if TIER_TWO so we can implement _PUSH_FRAME differently for both tiers.
1 parent 4f6f8f8 commit 6facc8d

File tree

8 files changed

+68
-28
lines changed

8 files changed

+68
-28
lines changed

Python/bytecodes.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2981,8 +2981,22 @@ dummy_func(
29812981
// (which will be pushed when the frame returns).
29822982
// It is needed so CALL_PY_EXACT_ARGS matches its family.
29832983
op(_PUSH_FRAME, (new_frame: _PyInterpreterFrame* -- unused)) {
2984+
// Write it out explicitly because it's subtly different.
2985+
// Eventually this should be the only occurrence of this code.
29842986
frame->return_offset = 0;
2985-
DISPATCH_INLINED(new_frame);
2987+
assert(tstate->interp->eval_frame == NULL);
2988+
SAVE_FRAME_STATE(); // Signals to the code generator
2989+
new_frame->previous = frame;
2990+
CALL_STAT_INC(inlined_py_calls);
2991+
#if TIER_ONE
2992+
frame = cframe.current_frame = new_frame;
2993+
goto start_frame;
2994+
#endif
2995+
#if TIER_TWO
2996+
frame = tstate->cframe->current_frame = new_frame;
2997+
stack_pointer = _PyFrame_GetStackPointer(frame);
2998+
ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;
2999+
#endif
29863000
}
29873001

29883002
macro(CALL_PY_EXACT_ARGS) =

Python/ceval.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
770770
#endif
771771
{
772772

773+
#define TIER_ONE 1
773774
#include "generated_cases.c.h"
774775

775776
/* INSTRUMENTED_LINE has to be here, rather than in bytecodes.c,

Python/ceval_macros.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,16 @@
103103
DISPATCH_GOTO(); \
104104
}
105105

106+
#define SAVE_FRAME_STATE() \
107+
do { \
108+
frame->prev_instr = next_instr - 1; \
109+
_PyFrame_SetStackPointer(frame, stack_pointer); \
110+
} while (0)
111+
106112
#define DISPATCH_INLINED(NEW_FRAME) \
107113
do { \
108114
assert(tstate->interp->eval_frame == NULL); \
109-
_PyFrame_SetStackPointer(frame, stack_pointer); \
110-
frame->prev_instr = next_instr - 1; \
115+
SAVE_FRAME_STATE(); \
111116
(NEW_FRAME)->previous = frame; \
112117
frame = cframe.current_frame = (NEW_FRAME); \
113118
CALL_STAT_INC(inlined_py_calls); \

Python/executor.c

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,12 @@
3030
#undef ENABLE_SPECIALIZATION
3131
#define ENABLE_SPECIALIZATION 0
3232

33-
#undef DISPATCH_INLINED
34-
#define DISPATCH_INLINED(NEW_FRAME) \
35-
do { \
36-
assert(tstate->interp->eval_frame == NULL); \
37-
_PyFrame_SetStackPointer(frame, stack_pointer); \
38-
frame->prev_instr -= 1; \
39-
(NEW_FRAME)->previous = frame; \
40-
frame = tstate->cframe->current_frame = (NEW_FRAME); \
41-
CALL_STAT_INC(inlined_py_calls); \
42-
stack_pointer = _PyFrame_GetStackPointer(frame); \
43-
ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; \
33+
#undef SAVE_FRAME_STATE
34+
#define SAVE_FRAME_STATE() \
35+
do { \
36+
/* Assume preceding SAVE_IP has set frame->prev_instr */ \
37+
frame->prev_instr--; \
38+
_PyFrame_SetStackPointer(frame, stack_pointer); \
4439
} while (0)
4540

4641

@@ -94,6 +89,7 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
9489
OBJECT_STAT_INC(optimization_uops_executed);
9590
switch (opcode) {
9691

92+
#define TIER_TWO 2
9793
#include "executor_cases.c.h"
9894

9995
default:

Python/executor_cases.c.h

Lines changed: 15 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/generated_cases.c.h

Lines changed: 15 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Tools/cases_generator/instructions.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class Instruction:
6060

6161
# Computed by constructor
6262
always_exits: str # If the block always exits, its last line; else ""
63+
save_frame_state: bool # Whether the instruction uses SAVE_FRAME_STATE()
6364
has_deopt: bool
6465
cache_offset: int
6566
cache_effects: list[parsing.CacheEffect]
@@ -83,6 +84,7 @@ def __init__(self, inst: parsing.InstDef):
8384
self.block
8485
)
8586
self.always_exits = always_exits(self.block_text)
87+
self.save_frame_state = variable_used(self.inst, "SAVE_FRAME_STATE")
8688
self.has_deopt = variable_used(self.inst, "DEOPT_IF")
8789
self.cache_effects = [
8890
effect for effect in inst.inputs if isinstance(effect, parsing.CacheEffect)
@@ -120,15 +122,13 @@ def __init__(self, inst: parsing.InstDef):
120122
def is_viable_uop(self) -> bool:
121123
"""Whether this instruction is viable as a uop."""
122124
dprint: typing.Callable[..., None] = lambda *args, **kwargs: None
123-
if "PY_EXACT" in self.name:
125+
if "PUSH_FRAME" in self.name:
124126
dprint = print
125127

126128
if self.name == "EXIT_TRACE":
127129
return True # This has 'return frame' but it's okay
128-
if self.name == "_PUSH_FRAME":
129-
return True # Has DISPATCH_INLINED but it's okay
130130
if self.always_exits:
131-
dprint(f"Skipping {self.name} because it always exits")
131+
dprint(f"Skipping {self.name} because it always exits: {self.always_exits}")
132132
return False
133133
if len(self.active_caches) > 1:
134134
# print(f"Skipping {self.name} because it has >1 cache entries")

Tools/cases_generator/stacking.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import dataclasses
22
import typing
33

4+
from flags import variable_used_unspecialized
45
from formatting import (
56
Formatter,
67
UNUSED,
@@ -314,7 +315,7 @@ def write_macro_instr(
314315
write_components(parts, out, TIER_ONE, mac.cache_offset)
315316
except AssertionError as err:
316317
raise AssertionError(f"Error writing macro {mac.name}") from err
317-
if not parts[-1].instr.always_exits:
318+
if not parts[-1].instr.always_exits and not parts[-1].instr.save_frame_state:
318319
if mac.cache_offset:
319320
out.emit(f"next_instr += {mac.cache_offset};")
320321
out.emit("DISPATCH();")
@@ -366,12 +367,7 @@ def write_components(
366367
poke.as_stack_effect(lax=True),
367368
)
368369

369-
dispatch_inlined_special_case = (
370-
mgr is managers[-1]
371-
and mgr.instr.always_exits.startswith("DISPATCH_INLINED")
372-
and mgr.instr.name == "_PUSH_FRAME"
373-
)
374-
if dispatch_inlined_special_case:
370+
if mgr.instr.save_frame_state:
375371
# Adjust stack to min_offset (input effects materialized)
376372
out.stack_adjust(mgr.min_offset.deep, mgr.min_offset.high)
377373
# Use clone() since adjust_inverse() mutates final_offset
@@ -385,7 +381,7 @@ def write_components(
385381
with out.block(""):
386382
mgr.instr.write_body(out, -4, mgr.active_caches, tier)
387383

388-
if mgr is managers[-1] and not dispatch_inlined_special_case:
384+
if mgr is managers[-1] and not mgr.instr.save_frame_state:
389385
# TODO: Explain why this adjustment is needed.
390386
out.stack_adjust(mgr.final_offset.deep, mgr.final_offset.high)
391387
# Use clone() since adjust_inverse() mutates final_offset

0 commit comments

Comments
 (0)