8000 GH-129709: Clean up tier two by brandtbucher · Pull Request #129710 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

GH-129709: Clean up tier two #129710

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
merged 5 commits into from
Feb 7, 2025
Merged

GH-129709: Clean up tier two #129710

merged 5 commits into from
Feb 7, 2025

Conversation

brandtbucher
Copy link
Member
@brandtbucher brandtbucher commented Feb 6, 2025 •
edited by bedevere-app bot
Loading

Sort of a grab-bag of improvements:

  • Silence some unused variable warnings in the tier two interpreter.
  • Replace the one remaining use of GOTO_ERROR(error) with ERROR_NO_POP().
  • Fix some bad format strings used for tier two debugging output.
  • Replace the handful of uses of GOTO_UNWIND with GOTO_TIER_ONE, replace EXIT_TO_TIER1 with GOTO_TIER_ONE, unify the goto_to_tier1, exit_to_tier1, and error_tier_two labels into the GOTO_TIER_ONE macro, and remove the unused exit_to_tier1_dynamic label and EXIT_TO_TIER1_DYNAMIC macro.
  • Replace separate Py_DECREF and NULLs of tstate->previous_executor with Py_CLEAR calls, since Py_DECREF escapes.
  • Move the refcounting to keep the first JIT executor alive from the shim frame into the GOTO_TIER_TWO macro.
  • Add a CURRENT_TARGET macro so the same return-to-tier-one logic can be shared by the interpreter and JIT.
  • Remove a bunch of unnecessary (void)item; lines from the abstract interpreter (these were ironically forcing the values to be loaded).
  • Mark _PyFrame_GetBytecode as non-escaping.
  • Use the same PATCH_VALUE implementation for the JIT templates and the shim frame.
  • Perform the same cleanups in the JIT that were done in the interpreter.

@brandtbucher brandtbucher self-assigned this Feb 6, 2025
@bedevere-app bedevere-app bot mentioned this pull request Feb 6, 2025
Open
@brandtbucher brandtbucher added skip news interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-JIT labels Feb 6, 2025
@markshannon
Copy link
Member

Replace separate Py_DECREF and NULLs of tstate->previous_executor with Py_CLEAR calls, since Py_DECREF escapes.

Py_CLEAR also escapes, so what does this do?

@brandtbucher
Copy link
Member Author

Py_CLEAR also escapes, so what does this do?

Py_CLEAR sets tstate->previous_executor = NULL before the decref, so there's no chance of re-entering the interpreter/JIT with a stale previous_executor. We assert that it's NULL in a bunch of places.

markshannon
markshannon approved these changes Feb 7, 2025
View reviewed changes
Copy link
Member
@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

One question, otherwise looks good.

@brandtbucher brandtbucher merged commit 70e387c into python:main Feb 7, 2025
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markshannon markshannon markshannon approved these changes

@Fidget-Spinner Fidget-Spinner Awaiting requested review from Fidget-Spinner Fidget-Spinner is a code owner

@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski savannahostrowski is a code owner

Assignees

@brandtbucher brandtbucher

Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) skip news topic-JIT
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@brandtbucher @markshannon
0