GH-132554: Fix tier2 FOR_ITER implementation and optimizations - #135137
GH-132554: Fix tier2 FOR_ITER implementation and optimizations#135137markshannon merged 4 commits into
Conversation
Sorry, something went wrong.
|
Can you check out the Windows JIT failures please? I'll review the PR after that. |
Sorry, something went wrong.
| #endif | ||
|
|
||
| _PyStackRef _PyForIter_NextWithIndex(PyObject *seq, _PyStackRef index); | ||
| _PyStackRef |
There was a problem hiding this comment.
| _PyStackRef | |
| PyAPI_FUNC(_PyStackRef) |
Sorry, something went wrong.
| op(_GET_ITER, (iterable -- iter, index_or_null)) { | ||
| if (sym_matches_type(iterable, &PyTuple_Type) || sym_matches_type(iterable, &PyList_Type)) { | ||
| iter = iterable; | ||
| index_or_null = sym_new_type(ctx, &PyLong_Type); |
There was a problem hiding this comment.
Hm, this is sort of weird. We don't have a symbol for "unboxed int" in the JIT, but it really doesn't feel correct to type this as int. Maybe leave as unknown and we can update our lattice with unboxed C types later? It's not like this information is being used yet, anyways.
| index_or_null = sym_new_type(ctx, &PyLong_Type); | |
| index_or_null = sym_new_unknown(ctx); |
Sorry, something went wrong.
| ERROR_NO_POP(); | ||
| } | ||
| next = PyStackRef_FromPyObjectSteal(next_o); | ||
| JUMPBY(oparg + 1); |
There was a problem hiding this comment.
Can you add back the comment that this is skipping the END_FOR?
| JUMPBY(oparg + 1); | |
| // Jump forward by oparg, then skip the following END_FOR: | |
| JUMPBY(oparg + 1); |
Sorry, something went wrong.
| if (PyStackRef_IsError(item)) { | ||
| ERROR_NO_POP(); | ||
| } | ||
| JUMPBY(oparg + 1); |
There was a problem hiding this comment.
| JUMPBY(oparg + 1); | |
| // Jump forward by oparg, then skip the following END_FOR: | |
| JUMPBY(oparg + 1); |
Sorry, something went wrong.
| _PyErr_Clear(tstate); | ||
| } | ||
| else { | ||
| return PyStackRef_ERROR; |
There was a problem hiding this comment.
| return PyStackRef_ERROR; | |
| return PyStackRef_ERROR; |
Sorry, something went wrong.
|
Confirmed that this fixed the pprint benchmarks locally. Just kicked off new benchmarks now. |
Sorry, something went wrong.
Sorry, something went wrong.
When adding virtual iterators, the tier 1 and tier 2 implementations of FOR_ITER diverged. I've already fixed a problem where the instrumented FOR_ITER differed from the normal one.
To prevent these problems happening again, this PR factors out the majority of FOR_ITER into a helper function for the 3 versions of FOR_ITER to share.
I've also added PyStackRef_ERROR to distinguish between errors and no result and remove the need for an additional out parameter for the helper function.
Also fixes a bug in the code generator where there are three or more output values, one is an unchanged input, one is a changed input and one is undefined.