Repository navigation
CALL_LIST_APPEND specialization can make no progress #141367
Description
Bug report
Bug description:
CALL_LIST_APPEND requires self to be a PyList_CheckExact in bytecodes.c. However, the specialize.c doesn't check that. This means there's a possible discrepancy between what we specialize for and what we deopt.
The fix is to check that self is a list. It should be as simple as
diff --git a/Python/specialize.c b/Python/specialize.c
index a1c5dedd615..5b44432b8b7 100644
--- a/Python/specialize.c
+++ b/Python/specialize.c
@@ -2041,8 +2041,12 @@ specialize_method_descriptor(PyMethodDescrObject *descr, _Py_CODEUNIT *instr,
bool pop = (next.op.code == POP_TOP);
int oparg = instr->op.arg;
if ((PyObject *)descr == list_append && oparg == 1 && pop) {
- specialize(instr, CALL_LIST_APPEND);
- return 0;
+ PyThreadState *tstate = PyThreadState_GET();
+ PyObject *self = PyStackRef_AsPyObjectBorrow(tstate->current_frame->stackpointer[-2]);
+ if (PyList_CheckExact(self)) {
+ specialize(instr, CALL_LIST_APPEND);
+ return 0;
+ }
}
specialize(instr, CALL_METHOD_DESCRIPTOR_O);
return 0;The test case should be in test_opcache.py, and test something like
class MyList(list): pass
x = MyList()
for _ in range(spec_counter):
x.append(1)and ensure that the append is not specialized to CALL_LIST_APPEND.
@efimov-mikhail can I let you take up this issue please? I'm pretty sure 3.14 is affected too.
CPython versions tested on:
CPython main branch, 3.15, 3.14
Operating systems tested on:
No response
Activity
efimov-mikhail commentedon Nov 10, 2025
@Fidget-Spinner , of course, I will take this, thanks.
efimov-mikhail commentedon Nov 11, 2025
Actually, it isn't affected.
You can see your own comment on PR with this change:
#134240 (comment)
So, basically, we should fix specialization on main branch only.
I'll create a PR soon.
Fidget-Spinner commentedon Nov 11, 2025
Great observation, thanks!
efimov-mikhail commentedon Nov 11, 2025
@Fidget-Spinner , I've created a PR with your proposed solution, CI is green.
I've used
_PyThreadState_GETinstead ofPyThreadState_GETbecause it's sligtly faster, IIUC.And make test for both
listandMyListappendspecialization.markshannon commentedon Nov 11, 2025
Rather than probing the stack
tstate->current_frame->stackpointer[-2], we should pass the arguments to_Py_Specialize_Call.We also need to add an additional guard in
_CALL_LIST_APPENDthatcallable == interp->callable_cache.list_append.efimov-mikhail commentedon Nov 11, 2025
Ok, I'll investigate this.
Do we really need this since
_GUARD_CALLABLE_LIST_APPENDis present?Maybe we should remove checking
DEOPT_IF(!PyList_CheckExact(self_o))in_GUARD_CALLABLE_LIST_APPENDbecause_GUARD_NOS_LISTis already used inCALL_LIST_APPENDmacro?markshannon commentedon Nov 11, 2025
No we don't. I was just looking at the uop, not the whole instruction.
Yes, that makes sense.
efimov-mikhail commentedon Nov 11, 2025
While working on this issue I've found this PR:
https://github.com/python/cpython/pull/132018/files#diff-af5c4ee62cca27529efe0febc3babfd5745cc1ae48a280a58605a8c85ee38942R335
Maybe we should change this test a little to use actual
SPECIALIZATION_THRESHOLDinstead of constant3?Fidget-Spinner commentedon Nov 11, 2025
Maybe not for this PR, but if you open another PR to fix it I'd be open to merging.
gh-141367: Use actual SPECIALIZATION_THRESHOLD value in specializatio…
gh-141367: Use CALL_LIST_APPEND instruction only for lists, not for l…
pythongh-141367: Use actual SPECIALIZATION_THRESHOLD value in special…
pythongh-141367: Use CALL_LIST_APPEND instruction only for lists, not…