gh-98831: Modernize the FOR_ITER family of instructions #101626
Conversation
gvanrossum
requested review from
iritkatriel,
Fidget-Spinner and
brandtbucher
| STAT_INC(FOR_ITER, hit); | ||
| PyListObject *seq = it->it_seq; | ||
| if (seq) { | ||
| if (it->it_index < PyList_GET_SIZE(seq)) { | ||
| PyObject *next = PyList_GET_ITEM(seq, it->it_index++); | ||
| PUSH(Py_NewRef(next)); | ||
| JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER); |
There was a problem hiding this comment.
I'm a little confused, why do we not need to JUMPBY INLINE_CACHE_ENTRIES_FOR_ITER anymore?
There was a problem hiding this comment.
I think this is the JUMPBY(1); in line 2688.
There was a problem hiding this comment.
Ahh right. I forgot that the inline cache entries for FOR_ITER is just the counter. Thanks Irit!
| FOR_ITER_GEN, | ||
| }; | ||
|
|
||
| inst(FOR_ITER, (unused/1, iter -- iter, next)) { |
There was a problem hiding this comment.
Should this be counter/1 instead of unused/1, and then the counter used in the body?
There was a problem hiding this comment.
In all cases where the counter is manipulated using ADAPTIVE_COUNTER_IS_ZERO / DECREMENT_ADAPTIVE_COUNTER I'm leaving this idiom unchanged -- the generator won't let you write back to cache entries, so it would become asymmetrical, and the common case is DECREMENT_ADAPTIVE_COUNTER anyway.
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
This is somewhat tricky because most of the family members (with the exception of FOR_ITER_GEN) are actually super-instructions that jump over the END_FOR instruction that is the target of the jump (which pops two stack items). In addition, FOR_ITER_RANGE also jumps over the STORE_FAST that follows it when the range is not exhausted.
The cache and stack effects documented in the DSL don't acknowledge the existence of these optimizations. (This will bite us when we try to incorporate these instructions in a level 2 interpreter, we'll have to deal with it then.)