Skip to content

GH-109214: Convert _SAVE_CURRENT_IP to _SET_IP in tier 2 trace creation. #110755

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 1 commit into from
Oct 12, 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
8 changes: 4 additions & 4 deletions Include/internal/pycore_opcode_metadata.h

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

4 changes: 0 additions & 4 deletions Python/abstract_interp_cases.c.h

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

12 changes: 2 additions & 10 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,6 @@ dummy_func(
}

macro(RETURN_VALUE) =
_SET_IP + // Tier 2 only; special-cased oparg
_SAVE_CURRENT_IP + // Sets frame->prev_instr
_POP_FRAME;

Expand All @@ -828,7 +827,6 @@ dummy_func(

macro(RETURN_CONST) =
LOAD_CONST +
_SET_IP + // Tier 2 only; special-cased oparg
_SAVE_CURRENT_IP + // Sets frame->prev_instr
_POP_FRAME;

Expand Down Expand Up @@ -3099,7 +3097,6 @@ dummy_func(
_CHECK_FUNCTION_EXACT_ARGS +
_CHECK_STACK_SPACE +
_INIT_CALL_PY_EXACT_ARGS +
_SET_IP + // Tier 2 only; special-cased oparg
_SAVE_CURRENT_IP + // Sets frame->prev_instr
_PUSH_FRAME;

Expand All @@ -3109,7 +3106,6 @@ dummy_func(
_CHECK_FUNCTION_EXACT_ARGS +
_CHECK_STACK_SPACE +
_INIT_CALL_PY_EXACT_ARGS +
_SET_IP + // Tier 2 only; special-cased oparg
_SAVE_CURRENT_IP + // Sets frame->prev_instr
_PUSH_FRAME;

Expand Down Expand Up @@ -3948,17 +3944,13 @@ dummy_func(
}

op(_SET_IP, (--)) {
TIER_TWO_ONLY
frame->prev_instr = ip_offset + oparg;
}

op(_SAVE_CURRENT_IP, (--)) {
#if TIER_ONE
TIER_ONE_ONLY
frame->prev_instr = next_instr - 1;
#endif
#if TIER_TWO
// Relies on a preceding _SET_IP
frame->prev_instr--;
#endif
}

op(_EXIT_TRACE, (--)) {
Expand Down
3 changes: 3 additions & 0 deletions Python/ceval_macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,9 @@ static inline void _Py_LeaveRecursiveCallPy(PyThreadState *tstate) {
/* Marker to specify tier 1 only instructions */
#define TIER_ONE_ONLY

/* Marker to specify tier 2 only instructions */
#define TIER_TWO_ONLY

/* Implementation of "macros" that modify the instruction pointer,
* stack pointer, or frame pointer.
* These need to treated differently by tier 1 and 2. */
Expand Down
12 changes: 1 addition & 11 deletions Python/executor_cases.c.h

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

28 changes: 4 additions & 24 deletions Python/generated_cases.c.h

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

8 changes: 5 additions & 3 deletions Python/optimizer.c
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ translate_bytecode_to_trace(
uint32_t orig_oparg = oparg; // For OPARG_TOP/BOTTOM
for (int i = 0; i < nuops; i++) {
oparg = orig_oparg;
uint32_t uop = expansion->uops[i].uop;
uint64_t operand = 0;
// Add one to account for the actual opcode/oparg pair:
int offset = expansion->uops[i].offset + 1;
Expand Down Expand Up @@ -680,6 +681,7 @@ translate_bytecode_to_trace(
break;
case OPARG_SET_IP: // op==_SET_IP; oparg=next instr
oparg = INSTR_IP(instr + offset, code);
uop = _SET_IP;
break;

default:
Expand All @@ -690,8 +692,8 @@ translate_bytecode_to_trace(
expansion->uops[i].offset);
Py_FatalError("garbled expansion");
}
ADD_TO_TRACE(expansion->uops[i].uop, oparg, operand);
if (expansion->uops[i].uop == _POP_FRAME) {
ADD_TO_TRACE(uop, oparg, operand);
if (uop == _POP_FRAME) {
TRACE_STACK_POP();
DPRINTF(2,
"Returning to %s (%s:%d) at byte offset %d\n",
Expand All @@ -701,7 +703,7 @@ translate_bytecode_to_trace(
2 * INSTR_IP(instr, code));
goto top;
}
if (expansion->uops[i].uop == _PUSH_FRAME) {
if (uop == _PUSH_FRAME) {
assert(i + 1 == nuops);
int func_version_offset =
offsetof(_PyCallCache, func_version)/sizeof(_Py_CODEUNIT)
Expand Down
6 changes: 3 additions & 3 deletions Tools/cases_generator/generate_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ def write_macro_expansions(
for part in parts:
if isinstance(part, Component):
# All component instructions must be viable uops
if not part.instr.is_viable_uop():
if not part.instr.is_viable_uop() and part.instr.name != "_SAVE_CURRENT_IP":
# This note just reminds us about macros that cannot
# be expanded to Tier 2 uops. It is not an error.
# It is sometimes emitted for macros that have a
Expand All @@ -671,8 +671,8 @@ def write_macro_expansions(
)
return
if not part.active_caches:
if part.instr.name == "_SET_IP":
size, offset = OPARG_SIZES["OPARG_SET_IP"], cache_offset
if part.instr.name == "_SAVE_CURRENT_IP":
size, offset = OPARG_SIZES["OPARG_SET_IP"], cache_offset - 1
else:
size, offset = OPARG_SIZES["OPARG_FULL"], 0
else:
Expand Down