GeezerVison is showing github.com — Specialization of some instructions does not conform to PEP 659, and prevents PEP 669 · Issue #100982 · python/cpython
Specialization of some instructions does not conform to PEP 659, and prevents PEP 669 #100982DescriptionAs written, PEP 659 says that individual specializations are restricted to a single instruction.
PEP 669 relies on this, as it also wants to replace instructions at runtime, and it would break if specialization occurs across multiple instructions.
Currently there are a two places where we break this design by specializing pairs of instructions together:
COMPARE_OP POP_JUMP_IF_ pairs are specialized togetherFOR_ITER STORE_FAST are specialized together
The second will go away with the register VM, and doesn't seem to be an issue in practice.
It is the COMPARE_OP POP_JUMP_IF_ specialization that is problematic, as PEP 669 wants to instrument branches.
Instrumenting the POP_JUMP_IF_ doesn't work if the COMPARE_OP specialization jumps over it.
The solution is to replace the COMPARE_OP POP_JUMP_IF_ pair with a single COMPARE_AND_BRANCH instruction that can be specialized or instrumented atomically.
Linked PRs
Activitymarkshannon commented on Jan 12, 2023 Superinstructions are also a problem for PEP 669, but are simpler to handle, and will also go away with the register VM. brandtbucher commented on Jan 18, 2023 BINARY_OP_INPLACE_ADD_UNICODE and CALL_NO_KW_LIST_APPEND are two more examples of "adaptive superinstructions".
Like FOR_ITER, though, they don't cross a branch and will probably disappear when we get a register VM. markshannon commented on Jan 27, 2023 changed the title [-]Specialization of `COMPARE_OP` does not conform to PEP 659, and prevents PEP 669[/-] [+]Specialization of some instructions does not conform to PEP 659, and prevents PEP 669[/+] on Jan 27, 2023markshannon commented on Jan 27, 2023 Although BINARY_OP_INPLACE_ADD_UNICODE and CALL_NO_KW_LIST_APPEND don't cross boundaries they may cross lines which interferes with instrumentation for LINE events. markshannon commented on Jan 27, 2023 The instruction that is skipped in CALL_NO_KW_LIST_APPEND is always a POP_TOP.
We can fix this by making sure that the POP_TOP following a call has no location. It probably should anyway.
The compiler already gives POP_TOP NO_LOCATION so nothing to do here
BINARY_OP_INPLACE_ADD_UNICODE is still a problem. It is rare, so the extra cost of de-specializing it during instrumentation and preventing that specialization if instrumentation is present would probably be OK.
Note:
This is only an issue for instrumentation of LINE events. INSTRUCTION events prevent all specialization, so there cannot be a conflict. markshannon commented on Jan 27, 2023 An alternative to fixing BINARY_OP_INPLACE_ADD_UNICODE is to restore the old behavior of BINARY_ADD to special case adding strings, and make sure that BINARY_OP specialization is so good that the performance of plain BINARY_ADD is irrelevant. markshannon commented on Jan 27, 2023 Which leaves FOR_ITER_RANGE. I'm hoping that improvements to freelists and integer representation will make the cost of creating the temporary int tolerable. added a commit that references this issue on Feb 22, 2023GH-100982: Restrict `FOR_ITER_RANGE` to a single instruction to allow… gvanrossum commented on May 17, 2023
FOR_ITER STORE_FAST are specialized together
The second will go away with the register VM, and doesn't seem to be an issue in practice.
Since there is no register VM (and probably won't be one, or not for a long time), what should we do about this? Ignore it because it "doesn't seem to be an issue in practice"?
OTOH maybe this issue can be closed, since it appears we do have PEP 669 after all? @markshannon markshannon commented on Jun 2, 2023 FOR_ITER and STORE_FAST are no longer specialized together (specifically for PEP 669): #101985
So, yes this can be closed. markshannon commented on Jun 19, 2023 It looks like we never fixed up BINARY_OP_INPLACE_ADD_UNICODE
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|