Skip to content

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

Merged
markshannon merged 8 commits intopython:mainfrom
faster-cpython:mark-decref-etc-as-escaping
Jan 13, 2025
Merged

GH-124483: Mark Py_DECREF, etc. as escaping for the JIT#128678
markshannon merged 8 commits intopython:mainfrom
faster-cpython:mark-decref-etc-as-escaping

Conversation

@markshannon
Copy link
Copy Markdown
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
Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@markshannon markshannon merged commit 39fc7ef into python:main Jan 13, 2025
@markshannon markshannon deleted the mark-decref-etc-as-escaping branch January 13, 2025 13:41
@brandtbucher
Copy link
Copy Markdown
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