Skip to content

Commit

Permalink
Most suggestions from Mark's code review
Browse files Browse the repository at this point in the history
- Remove CHECK_EVAL_BREAKER() from top of Tier 2 loop
- Make Tier 2 default case Py_UNREACHABLE() in non-debug mode
- GOTO_TIER_TWO() macro instead of `goto enter_tier_two`
- Inline ip_offset (Tier 2 LOAD_IP() is now a no-op)
- Move #define/#under TIER_ONE/TIER_TWO into generated .c.h files
- ceval_macros.h defines Tier 1 macros, Tier 2 is inlined in ceval.c
  • Loading branch information
gvanrossum committed Oct 31, 2023
1 parent a0aed59 commit 75605c7
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 33 deletions.
5 changes: 3 additions & 2 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -2314,7 +2314,7 @@ dummy_func(
Py_INCREF(executor);
if (executor->execute == _PyUopExecute) {
self = (_PyUOpExecutorObject *)executor;
goto enter_tier_two;
GOTO_TIER_TWO();
}
frame = executor->execute(executor, frame, stack_pointer);
if (frame == NULL) {
Expand Down Expand Up @@ -3942,7 +3942,8 @@ dummy_func(

op(_SET_IP, (--)) {
TIER_TWO_ONLY
frame->instr_ptr = ip_offset + oparg;
// TODO: Put the code pointer in `operand` to avoid indirection via `frame`
frame->instr_ptr = _PyCode_CODE(_PyFrame_GetCode(frame)) + oparg;
}

op(_SAVE_RETURN_OFFSET, (--)) {
Expand Down
14 changes: 5 additions & 9 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,10 +611,8 @@ PyEval_EvalFrameEx(PyFrameObject *f, int throwflag)
return _PyEval_EvalFrame(tstate, f->f_frame, throwflag);
}

#define TIER_ONE 1
#include "ceval_macros.h"


int _Py_CheckRecursiveCallPy(
PyThreadState *tstate)
{
Expand Down Expand Up @@ -952,8 +950,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
enter_tier_two:

#undef LOAD_IP
#define LOAD_IP(UNUSED) \
do { ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; } while (0)
#define LOAD_IP(UNUSED) (void)0

#undef GOTO_ERROR
#define GOTO_ERROR(LABEL) goto LABEL ## _tier_two
Expand Down Expand Up @@ -984,10 +981,7 @@ do { ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; } wh
#define DPRINTF(level, ...)
#endif

CHECK_EVAL_BREAKER();

OPT_STAT_INC(traces_executed);
_Py_CODEUNIT *ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive;
_PyUOpInstruction *next_uop = self->trace;
uint64_t operand;
#ifdef Py_STATS
Expand All @@ -1013,16 +1007,18 @@ do { ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; } wh

switch (opcode) {

#undef TIER_ONE
#define TIER_TWO 2
#include "executor_cases.c.h"

default:
#ifdef Py_DEBUG
{
fprintf(stderr, "Unknown uop %d, oparg %d, operand %" PRIu64 "\n",
opcode, oparg, operand);
Py_FatalError("Unknown uop");
}
#else
Py_UNREACHABLE();
#endif

}
}
Expand Down
25 changes: 5 additions & 20 deletions Python/ceval_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,36 +377,21 @@ static inline void _Py_LeaveRecursiveCallPy(PyThreadState *tstate) {

/* Implementation of "macros" that modify the instruction pointer,
* stack pointer, or frame pointer.
* These need to treated differently by tier 1 and 2. */

#if TIER_ONE
* These need to treated differently by tier 1 and 2.
* The Tier 1 version is here; Tier 2 is inlined in ceval.c. */

#define LOAD_IP(OFFSET) do { \
next_instr = frame->instr_ptr + (OFFSET); \
} while (0)

#define STORE_SP() \
_PyFrame_SetStackPointer(frame, stack_pointer)

#define LOAD_SP() \
stack_pointer = _PyFrame_GetStackPointer(frame);

#endif


#if TIER_TWO

#define LOAD_IP(UNUSED) \
do { ip_offset = (_Py_CODEUNIT *)_PyFrame_GetCode(frame)->co_code_adaptive; } while (0)
/* There's no STORE_IP(), it's inlined by the code generator. */

#define STORE_SP() \
_PyFrame_SetStackPointer(frame, stack_pointer)

#define LOAD_SP() \
stack_pointer = _PyFrame_GetStackPointer(frame);

#endif



/* Tier-switching macros. */

#define GOTO_TIER_TWO() goto enter_tier_two;
10 changes: 9 additions & 1 deletion Python/executor_cases.c.h

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

9 changes: 8 additions & 1 deletion Python/generated_cases.c.h

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

20 changes: 20 additions & 0 deletions Tools/cases_generator/generate_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,12 @@ def write_instructions(

self.write_provenance_header()

self.out.write_raw("\n")
self.out.write_raw("#ifdef TIER_TWO\n")
self.out.write_raw(" #error \"This file is for Tier 1 only\"\n")
self.out.write_raw("#endif\n")
self.out.write_raw("#define TIER_ONE 1\n")

# Write and count instructions of all kinds
n_macros = 0
for thing in self.everything:
Expand All @@ -773,6 +779,9 @@ def write_instructions(
case _:
assert_never(thing)

self.out.write_raw("\n")
self.out.write_raw("#undef TIER_ONE\n")

print(
f"Wrote {n_macros} cases to {output_filename}",
file=sys.stderr,
Expand All @@ -786,6 +795,13 @@ def write_executor_instructions(
with open(executor_filename, "w") as f:
self.out = Formatter(f, 8, emit_line_directives)
self.write_provenance_header()

self.out.write_raw("\n")
self.out.write_raw("#ifdef TIER_ONE\n")
self.out.write_raw(" #error \"This file is for Tier 2 only\"\n")
self.out.write_raw("#endif\n")
self.out.write_raw("#define TIER_TWO 2\n")

for instr in self.instrs.values():
if instr.is_viable_uop():
n_uops += 1
Expand All @@ -795,6 +811,10 @@ def write_executor_instructions(
if instr.check_eval_breaker:
self.out.emit("CHECK_EVAL_BREAKER();")
self.out.emit("break;")

self.out.write_raw("\n")
self.out.write_raw("#undef TIER_TWO\n")

print(
f"Wrote {n_uops} cases to {executor_filename}",
file=sys.stderr,
Expand Down

0 comments on commit 75605c7

Please sign in to comment.