Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
gvanrossum merged 13 commits into from
Jul 14, 2023
Merged

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

gvanrossum merged 13 commits into from
Jul 14, 2023

Conversation

@gvanrossum
Copy link
Member

@gvanrossumgvanrossum commented Jul 12, 2023•
edited
Loading

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
Copy link
MemberAuthor

gvanrossum commented Jul 12, 2023

@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
Python/optimizer.c
_Py_CODEUNIT*initial_instr=instr;
inttrace_length=0;
intmax_length=buffer_size;
intreserved=0;
Copy link
Member

@iritkatrieliritkatrielJul 13, 2023

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?

Copy link
MemberAuthor

@gvanrossumgvanrossumJul 13, 2023

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
Copy link
MemberAuthor

gvanrossum commented Jul 13, 2023

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

@gvanrossumgvanrossum merged commit 025995f into python:mainJul 14, 2023
@gvanrossumgvanrossum 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