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-116422: Modify a few uops so that they can be supported by tier 2 with hot/cold splitting #116832

Merged
merged 2 commits into from
Mar 15, 2024

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Mar 14, 2024

This PR unifies the error handling of a few uops so that they either use only EXIT_IF or only GOTO_ERROR.

@@ -138,12 +138,12 @@ extern void _PyEval_DeactivateOpCache(void);
/* With USE_STACKCHECK macro defined, trigger stack checks in
_Py_CheckRecursiveCall() on every 64th call to _Py_EnterRecursiveCall. */
static inline int _Py_MakeRecCheck(PyThreadState *tstate) {
return (tstate->c_recursion_remaining-- <= 0
return (tstate->c_recursion_remaining-- < 0
Copy link
Member Author

Choose a reason for hiding this comment

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

The reason this change is here is to make the guard tstate->c_recursion_remaining <= 0 instead of tstate->c_recursion_remaining <= 1. Comparing to zero is generally faster.

It also seems more correct. If c_recursion_remaining is 1, that suggest that there is 1 remaining call, not 0.

Copy link
Member Author

Choose a reason for hiding this comment

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

The actual number we compare to doesn't really matter, as long as we are consistent.

@@ -3616,13 +3614,11 @@ dummy_func(
PyObject *self = args[0];
DEOPT_IF(!Py_IS_TYPE(self, method->d_common.d_type));
DEOPT_IF(meth->ml_flags != METH_NOARGS);
// CPython promises to check all non-vectorcall function calls.
DEOPT_IF(tstate->c_recursion_remaining <= 0);
Copy link
Member

Choose a reason for hiding this comment

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

The idea is that after DEOPT we will end up doing GOTO_ERROR in the original bytecode? (Is that what the comment is saying?)

Copy link
Member Author

Choose a reason for hiding this comment

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

The comment refers to our obligation to check the C recursion limit for almost all builtin function calls.

We can fulfil that obligation more cheaply this way.

@markshannon markshannon merged commit 2cf18a4 into python:main Mar 15, 2024
53 of 54 checks passed
vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@markshannon markshannon deleted the unify-errors-in-calls branch August 6, 2024 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants