Skip to content

gh-106529: Split FOR_ITER_{LIST,TUPLE} into uops - #106696

Merged
gvanrossum merged 13 commits into
python:mainfrom
gvanrossum:for-iter-list-tuple
Jul 14, 2023
Merged

gh-106529: Split FOR_ITER_{LIST,TUPLE} into uops#106696
gvanrossum merged 13 commits into
python:mainfrom
gvanrossum:for-iter-list-tuple

Conversation

@gvanrossum

@gvanrossumgvanrossum commented Jul 12, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Same recipe as FOR_ITER_RANGE. I refactored the Tier 2 transformation code to handle similar cases. I had to fix a bug in the space reservation code, and decided to add some macros and an extra guard rail for that.

@bedevere-botbedevere-bot mentioned this pull request Jul 12, 2023
Closed
11 tasks
@gvanrossum

gvanrossum commented Jul 12, 2023

Copy link
Copy Markdown
MemberAuthor

@iritkatriel Do you have time to look at the reservation code? The rest is on auto-pilot (I'm going to add FOR_ITER_TUPLE to the same PR.

@gvanrossumgvanrossum changed the title gh-106529: Split FOR_ITER_LIST into uopsgh-106529: Split FOR_ITER_{LIST,TUPLE} into uopsJul 12, 2023
@gvanrossumgvanrossum mentioned this pull request Jul 13, 2023
Closed
iritkatriel
iritkatriel reviewed Jul 13, 2023
View reviewed changes
Comment threadPython/bytecodes.c Outdated
Comment threadPython/bytecodes.c Outdated
Comment threadPython/bytecodes.c Outdated
Comment threadPython/optimizer.c Outdated
Comment threadPython/optimizer.c
_Py_CODEUNIT *initial_instr = instr;
int trace_length = 0;
int max_length = buffer_size;
int reserved = 0;

@iritkatrieliritkatrielJul 13, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point of reserve is to make sure that you won't error out in the middle of an opcode's translation to uops?

@gvanrossumgvanrossumJul 13, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I'd rather reserve space ahead than having to bail out in the middle and undo some work already done (the undoing feels more brittle).

@gvanrossum

gvanrossum commented Jul 13, 2023

Copy link
Copy Markdown
MemberAuthor

Assuming the tests pass this time I'll just merge.

@gvanrossum
gvanrossum merged commit 025995f into python:mainJul 14, 2023
@gvanrossum
gvanrossum deleted the for-iter-list-tuple branch July 14, 2023 00:27
kgdiem pushed a commit to kgdiem/cpython that referenced this pull request Jul 14, 2023
@gvanrossum@kgdiem
pythongh-106529: Split FOR_ITER_{LIST,TUPLE} into uops (python#106696)
c15f572
Also rename `_ITER_EXHAUSTED_XXX` to `_IS_ITER_EXHAUSTED_XXX` to make it clear this is a test.
@gvanrossumgvanrossum mentioned this pull request Jul 14, 2023
Merged
gvanrossum added a commit that referenced this pull request Jul 15, 2023
@gvanrossum
gh-106529: Fix subtle Tier 2 edge case with list iterator (#106756)
0db85ee
The Tier 2 opcode _IS_ITER_EXHAUSTED_LIST (and _TUPLE)
didn't set it->it_seq to NULL, causing a subtle bug
that resulted in test_exhausted_iterator in list_tests.py
to fail when running all tests with -Xuops.
The bug was introduced in gh-106696.
Added this as an explicit test.
Also fixed the dependencies for ceval.o -- it depends on executor_cases.c.h.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatrieliritkatrieliritkatriel left review comments
@markshannonmarkshannonmarkshannon left review comments

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gvanrossum@iritkatriel@markshannon@bedevere-bot