Skip to content

gh-106529: Make FOR_ITER a viable uop #112134

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 17 commits into from
Nov 20, 2023
Merged
Show file tree
Hide file tree
Changes from 5 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
86 changes: 47 additions & 39 deletions Include/internal/pycore_opcode_metadata.h

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

1 change: 1 addition & 0 deletions Makefile.pre.in
Original file line number Diff line number Diff line change
Expand Up @@ -1609,6 +1609,7 @@ Python/ceval.o: \
$(srcdir)/Python/ceval_macros.h \
$(srcdir)/Python/condvar.h \
$(srcdir)/Python/generated_cases.c.h \
$(srcdir)/Python/executor_cases.c.h \
$(srcdir)/Python/opcode_targets.h

Python/flowgraph.o: \
Expand Down
10 changes: 10 additions & 0 deletions Python/abstract_interp_cases.c.h

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

29 changes: 27 additions & 2 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -2368,7 +2368,7 @@ dummy_func(
goto enter_tier_one;
}

replaced op(_POP_JUMP_IF_FALSE, (unused/1, cond -- )) {
replaced op(_POP_JUMP_IF_FALSE, (unused/1, cond -- )) {
assert(PyBool_Check(cond));
int flag = Py_IsFalse(cond);
#if ENABLE_SPECIALIZATION
Expand Down Expand Up @@ -2512,7 +2512,7 @@ dummy_func(
#endif /* ENABLE_SPECIALIZATION */
}

op(_FOR_ITER, (iter -- iter, next)) {
replaced op(_FOR_ITER, (iter -- iter, next)) {
/* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */
next = (*Py_TYPE(iter)->tp_iternext)(iter);
if (next == NULL) {
Expand All @@ -2535,6 +2535,31 @@ dummy_func(
// Common case: no jump, leave it to the code generator
}

op(_FOR_ITER_TIER_TWO, (iter -- iter, next)) {
/* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */
next = (*Py_TYPE(iter)->tp_iternext)(iter);
if (next == NULL) {
if (_PyErr_Occurred(tstate)) {
if (!_PyErr_ExceptionMatches(tstate, PyExc_StopIteration)) {
GOTO_ERROR(error);
}
_PyErr_Clear(tstate);
}
/* iterator ended normally */
Py_DECREF(iter);
STACK_SHRINK(1);
/* HACK: Emulate DEOPT_IF to jump over END_FOR */
Copy link
Member

Choose a reason for hiding this comment

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

No hacks, please 🙂

The code should look like this:

if (next == NULL) {
    if (_PyErr_Occurred(tstate)) {
        if (!_PyErr_ExceptionMatches(tstate, PyExc_StopIteration)) {
            GOTO_ERROR(error);
        }
        _PyErr_Clear(tstate);
    }
    /* iterator ended normally */
    Py_DECREF(iter);
    STACK_SHRINK(1);
    DEOPT_IF(true);
}

The trace generator can adjust the target, so it points after the END_FOR.

_PyFrame_SetStackPointer(frame, stack_pointer);
frame->instr_ptr += 1 + INLINE_CACHE_ENTRIES_FOR_ITER + oparg + 1;
assert(frame->instr_ptr[-1].op.code == END_FOR ||
frame->instr_ptr[-1].op.code == INSTRUMENTED_END_FOR);
Py_DECREF(current_executor);
OPT_HIST(trace_uop_execution_counter, trace_run_length_hist);
goto enter_tier_one;
}
// Common case: no jump, leave it to the code generator
}

macro(FOR_ITER) = _SPECIALIZE_FOR_ITER + _FOR_ITER;

inst(INSTRUMENTED_FOR_ITER, (unused/1 -- )) {
Expand Down
49 changes: 49 additions & 0 deletions Python/executor_cases.c.h

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

22 changes: 21 additions & 1 deletion Python/optimizer.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,8 @@ uop_dealloc(_PyUOpExecutorObject *self) {
}

static const char *
uop_name(int index) {
uop_name(int index)
{
if (index <= MAX_REAL_OPCODE) {
return _PyOpcode_OpName[index];
}
Expand Down Expand Up @@ -391,6 +392,7 @@ _PyUop_Replacements[OPCODE_METADATA_SIZE] = {
[_ITER_JUMP_RANGE] = _GUARD_NOT_EXHAUSTED_RANGE,
[_ITER_JUMP_LIST] = _GUARD_NOT_EXHAUSTED_LIST,
[_ITER_JUMP_TUPLE] = _GUARD_NOT_EXHAUSTED_TUPLE,
[_FOR_ITER] = _FOR_ITER_TIER_TWO,
};

static const uint16_t
Expand Down Expand Up @@ -832,6 +834,24 @@ make_executor_from_uops(_PyUOpInstruction *buffer, _PyBloomFilter *dependencies)
assert(dest == -1);
executor->base.execute = _PyUopExecute;
_Py_ExecutorInit((_PyExecutorObject *)executor, dependencies);
#ifdef Py_DEBUG
char *python_lltrace = Py_GETENV("PYTHON_LLTRACE");
int lltrace = 0;
if (python_lltrace != NULL && *python_lltrace >= '0') {
lltrace = *python_lltrace - '0'; // TODO: Parse an int and all that
}
if (lltrace >= 2) {
printf("Optimized executor (length %d):\n", length);
for (int i = 0; i < length; i++) {
printf("%4d %s(%d, %d, %" PRIu64 ")\n",
i,
uop_name(executor->trace[i].opcode),
executor->trace[i].oparg,
executor->trace[i].target,
executor->trace[i].operand);
}
}
#endif
return (_PyExecutorObject *)executor;
}

Expand Down
9 changes: 4 additions & 5 deletions Tools/cases_generator/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@

def makes_escaping_api_call(instr: parsing.InstDef) -> bool:
if "CALL_INTRINSIC" in instr.name:
return True;
return True
tkns = iter(instr.tokens)
for tkn in tkns:
if tkn.kind != lx.IDENTIFIER:
Expand All @@ -79,6 +79,7 @@ def makes_escaping_api_call(instr: parsing.InstDef) -> bool:
return True
return False


@dataclasses.dataclass
class InstructionFlags:
"""Construct and manipulate instruction flags"""
Expand Down Expand Up @@ -124,9 +125,7 @@ def fromInstruction(instr: parsing.InstDef) -> "InstructionFlags":
or variable_used(instr, "exception_unwind")
or variable_used(instr, "resume_with_error")
),
HAS_ESCAPES_FLAG=(
makes_escaping_api_call(instr)
),
HAS_ESCAPES_FLAG=makes_escaping_api_call(instr),
)

@staticmethod
Expand Down Expand Up @@ -176,7 +175,7 @@ def variable_used_unspecialized(node: parsing.Node, name: str) -> bool:
tokens: list[lx.Token] = []
skipping = False
for i, token in enumerate(node.tokens):
if token.kind == "MACRO":
if token.kind == "CMACRO":
Copy link
Member Author

@gvanrossum gvanrossum Nov 16, 2023

Choose a reason for hiding this comment

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

NOTE: This fix resulted in _SPECIALIZE_UNPACK_SEQUENCE becoming a viable uop. It was missing a TIER_ONE_ONLY marker; I've added it back. (The fix is needed to restore the feature that this function doesn't look inside #if TIER_ONE.)

text = "".join(token.text.split())
# TODO: Handle nested #if
if text == "#if":
Expand Down
2 changes: 1 addition & 1 deletion 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(
if not part.instr.is_viable_uop() and "replaced" not in part.instr.annotations:
# 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
# Suppress it using 'replaced op(...)' for macros having
# manual translation in translate_bytecode_to_trace()
# in Python/optimizer.c.
if len(parts) > 1 or part.instr.name != name:
Expand Down
2 changes: 1 addition & 1 deletion Tools/cases_generator/instructions.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def __init__(self, inst: parsing.InstDef):
def is_viable_uop(self) -> bool:
"""Whether this instruction is viable as a uop."""
dprint: typing.Callable[..., None] = lambda *args, **kwargs: None
if "FRAME" in self.name:
if self.name == "_FOR_ITER_TIER_TWO":
dprint = print

if self.name == "_EXIT_TRACE":
Expand Down
4 changes: 2 additions & 2 deletions Tools/cases_generator/parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class OpName(Node):

@dataclass
class InstHeader(Node):
annotations : list[str]
annotations: list[str]
kind: Literal["inst", "op"]
name: str
inputs: list[InputEffect]
Expand All @@ -114,7 +114,7 @@ class InstHeader(Node):

@dataclass
class InstDef(Node):
annotations : list[str]
annotations: list[str]
kind: Literal["inst", "op"]
name: str
inputs: list[InputEffect]
Expand Down