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

GH-124483: Mark Py_DECREF, etc. as escaping for the JIT #128678

Merged
merged 8 commits into from
Jan 13, 2025

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Jan 9, 2025

Unlike #124615 this doesn't change tier 1 or GC behavior at all.
This ensures correctness of tier 2 without any potential breakage.

We will want to handle Py_DECREF etc. in a more sophisticated way in the future, to improve GC and enable refcounting optimizations. For now though, this fixes the bug in tier2.

@markshannon markshannon changed the title Mark Py_DECREF, etc. as escaping for tier 1 GH-124483: Mark Py_DECREF, etc. as escaping for tier 1 Jan 9, 2025
@markshannon markshannon changed the title GH-124483: Mark Py_DECREF, etc. as escaping for tier 1 GH-124483: Mark Py_DECREF, etc. as escaping for the JIT Jan 9, 2025
@markshannon markshannon merged commit 39fc7ef into python:main Jan 13, 2025
49 checks passed
@markshannon markshannon deleted the mark-decref-etc-as-escaping branch January 13, 2025 13:41
@brandtbucher
Copy link
Member

@markshannon, I think we also need to handle DECREF_INPUTS (_POP_TOP should be escaping, for example), as well as steal-y stuff:

  • _PyTuple_FromStackRefSteal
  • _PyList_FromStackRefSteal
  • ...am I missing anything else?

An alternative that we discussed is to make the steal-y things only steal on success, and we count on the error handler to pop things otherwise., since it would be a shame if returning a tuple was an "escaping" operation. We could probably also do something similar for _PyEvalFramePushAndInit, if so.

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.

3 participants