GH-118095: Unify the behavior of tier 2 FOR_ITER branch micro-ops#118420
Uh oh!
There was an error while loading. Please reload this page.
Conversation
markshannon changed the title FOR_ITER branch micro-opsFOR_ITER branch micro-ops
…prepare for execution step.
…and fix off by one error)
markshannonforce-pushed the unify-tier-2-for-iter branch from 25a889a to bb7efd4Comparemarkshannon commented May 1, 2024
The stats are a bit confusing. The number of traces executed goes up, but the number of uops executed goes down, as we would expect. Looking at the number of instructions executed for the various tier 1 and tier 2 Specialized
Unspecialized
Specialization is being improved: we are executing more specialized T1 and T2 variants and much fewer unspecialized |
brandtbucher commented May 1, 2024
The test hangs for tier two seem to be in |
gvanrossum left a comment
gvanrossum left a comment There was a problem hiding this comment.
Seem to be some unrelated cleanups -- maybe minimize those or extract them to another PR? Otherwise LGTM.
Python/optimizer.c Outdated
| #defineMAX_EXECUTORS_SIZE 256 | ||
| #ifdefPy_DEBUG | ||
| staticintbase_opcode(PyCodeObject*code, intoffset) |
There was a problem hiding this comment.
| staticintbase_opcode(PyCodeObject*code, intoffset) | |
| staticint | |
| base_opcode(PyCodeObject*code, intoffset) |
| if (_Py_uop_sym_is_not_null(sym)){ | ||
| sym_set_bottom(sym); | ||
| return false; | ||
| } | ||
| sym_set_flag(sym, IS_NULL); | ||
| return!_Py_uop_sym_is_bottom(sym); | ||
| returntrue; |
There was a problem hiding this comment.
Does this refactoring matter? If so, why not do the same for set_non_null below?
There was a problem hiding this comment.
It is not a refactoring.
Calling _Py_uop_sym_set_null on a non-NULL symbol would fail an assertion in _Py_uop_sym_is_bottom
And yes, it should be applied to set_non_null as well.
| #ifdefPy_DEBUG | ||
| uint32_tnext_inst=target+1+INLINE_CACHE_ENTRIES_FOR_ITER+ (oparg>255); | ||
| uint32_tjump_target=next_inst+oparg; | ||
| assert(base_opcode(code, jump_target) ==END_FOR|| | ||
| base_opcode(code, jump_target) ==INSTRUMENTED_END_FOR); | ||
| assert(base_opcode(code, jump_target+1) ==POP_TOP); | ||
| #endif |
There was a problem hiding this comment.
Probably best to include the case also in {... } to limit the scope of the two variables declared in debug mode.
…ps (pythonGH-118420) * Target _FOR_ITER_TIER_TWO at POP_TOP following the matching END_FOR * Modify _GUARD_NOT_EXHAUSTED_RANGE, _GUARD_NOT_EXHAUSTED_LIST and _GUARD_NOT_EXHAUSTED_TUPLE so that they also target the POP_TOP following the matching END_FOR
3 participants
Simplifies and unifies the behavior of
_GUARD_NOT_EXHAUSTED_RANGE_GUARD_NOT_EXHAUSTED_LIST_GUARD_NOT_EXHAUSTED_TUPLE_FOR_ITER_TIER_TWOSuch that all leave just the iterator on the stack and they exit to the
POP_TOPimmediately after the associatedEND_FOR.This fixes a bug in the tier 2 handling of
_FOR_ITER_TIER_TWOwhere errors were treated as occurring at the jump target, not an the original instructions.