Skip to content
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.

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

Kenjin tier2 inliner redux #116394

Closed
wants to merge 28 commits into from

Conversation

gvanrossum
Copy link
Member

Like Fidget-Spinner#72 but without main merge.

Python/ceval.c Outdated
Comment on lines 1678 to 1681
// Make sure that this is, indeed, the top frame. We can't check this in
// _PyThreadState_PopFrame, since f_code is already cleared at that point:
assert((PyObject **)frame + _PyFrame_GetCode(frame)->co_framesize ==
tstate->datastack_top);
// // Make sure that this is, indeed, the top frame. We can't check this in
// // _PyThreadState_PopFrame, since f_code is already cleared at that point:
// assert((PyObject **)frame + _PyFrame_GetCode(frame)->co_framesize ==
// tstate->datastack_top);
Copy link
Member Author

Choose a reason for hiding this comment

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

Obviously this block should just be deleted. I'm leaving it in because it seems a kind of important check, but I don't know how to reproduce it now that datastack_top may be larger than the end of the reserved stack space due to call inlining.

@gvanrossum gvanrossum force-pushed the kenjin_tier2_inliner_redux branch from 6c63c21 to dad9100 Compare March 6, 2024 00:19
@gvanrossum gvanrossum force-pushed the kenjin_tier2_inliner_redux branch from 396223a to 2f9539b Compare March 6, 2024 02:05
@gvanrossum
Copy link
Member Author

@Fidget-Spinner Could you merge this in your PR's branch?

@gvanrossum
Copy link
Member Author

Closing,nNow that this is merged into gh-116290.

@gvanrossum gvanrossum closed this Mar 7, 2024
@gvanrossum gvanrossum deleted the kenjin_tier2_inliner_redux branch March 19, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants