gh-148874: add flag to CALL to skip periodic check in with#149019
gh-148874: add flag to CALL to skip periodic check in with#149019NekoAsakura wants to merge 5 commits intopython:mainfrom
CALL to skip periodic check in with#149019Conversation
NekoAsakura
requested review from
Fidget-Spinner,
ericsnowcurrently,
iritkatriel,
markshannon,
savannahostrowski and
tomasr8
as code owners
markshannon
left a comment
markshannon
left a comment
There was a problem hiding this comment.
This looks good in general, but I've a few comments.
I'd prefer explicit oparg >> 1 and oparg & 1 so the tools can more easily understand offsets.
| Calls with keyword arguments are now handled by :opcode:`CALL_KW`. | ||
|
|
||
| .. versionchanged:: next | ||
| ``argc`` is now the oparg shifted right by one. |
There was a problem hiding this comment.
Also explain what the low bit is for.
| #define RESUME_OPARG_LOCATION_MASK 0x7 | ||
| #define RESUME_OPARG_DEPTH1_MASK 0x8 | ||
|
|
||
| #define CALL_OPARG_SKIP_PENDING_MASK 0x1 |
There was a problem hiding this comment.
Can you avoid using macros (or inline functions) for this?
This is sp that our tools can understand the offsets in calls.
The tools can (with a bit of modification) understand oparg >> 1, but CALL_ARGCOUNT will require special casing CALL_ARGCOUNT making it more complex.
(The reason this matters is that we will want to convert fixes sized arrays to scalars in the code generator in the future)
Also, comments anywhere there is masking or shifting would be clearer, IMO, than opaque macros.
| elif deop in (CALL, INSTRUMENTED_CALL): | ||
| argval = arg >> 1 | ||
| if arg & 1: | ||
| argrepr = f"{argval} + skip_pending" |
There was a problem hiding this comment.
We use the term "no interrupt" for jumps, so could you use that here for consistency?
| ERROR_IF(err != 0); | ||
| } | ||
|
|
||
| replaced op(_CHECK_PERIODIC_AT_END_PENDING, (--)) { |
There was a problem hiding this comment.
| replaced op(_CHECK_PERIODIC_AT_END_PENDING, (--)) { | |
| replaced op(_CHECK_PERIODIC_IF_INTERRUPTIBLE, (--)) { |
The meaning of _CHECK_PERIODIC_AT_END_PENDING isn't clear.
| [_FOR_ITER_VIRTUAL] = _FOR_ITER_VIRTUAL_TIER_TWO, | ||
| [_ITER_NEXT_LIST] = _ITER_NEXT_LIST_TIER_TWO, | ||
| [_CHECK_PERIODIC_AT_END] = _TIER2_RESUME_CHECK, | ||
| [_CHECK_PERIODIC_AT_END_PENDING] = _TIER2_RESUME_CHECK, |
There was a problem hiding this comment.
This should either be replaced by _NOP if oparg & 1 is 0 or _CHECK_PERIODIC_AT_END if oparg & 1 is 1 in the front-end, so I wouldn't expect it to appear here.
We do much the same with _PUSH_NULL_CONDITIONAL, converting it to _NOP or _PUSH_NULL
Documentation build overview
33 files changed ·
|
|
Thanks for the review <3 |
|
A bit of clash with #148831, having a look. |
2 participants
with lock: can skip__exit__when a signal handler raises right after__enter__#148874📚 Documentation preview 📚: https://cpython-previews--149019.org.readthedocs.build/