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-120024: Remove CHECK_EVAL_BREAKER macro. #122968

Merged
merged 8 commits into from
Aug 14, 2024

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Aug 13, 2024

Removing the CHECK_EVAL_BREAKER by moving into its own pair of micro-ops allows us to remove a lot of special case code that impairs getting escaping calls.
The CHECK_EVAL_BREAKER macro is a conditional expression containing an escaping call and error handling. By removing the macro, the code generator can see the parts and analyze them correctly.

This PR is probably best reviewed commit by commit.

Python/bytecodes.c Outdated Show resolved Hide resolved
Python/bytecodes.c Outdated Show resolved Hide resolved
if (_Py_HandlePending(tstate) != 0) {
GOTO_ERROR(error); \
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can this be implemented as a macro that reuses _CHECK_PERIODIC? Like "if condition is false, exit, then do _CHECK_PERIODIC"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removing the macro is the point of this PR. Macros are opaque to the code generator, so we need to special case them (which is OK for something very common like Py_DECREF). Avoiding the special case simplifies the code generator considerably.
It is the call to _Py_HandlePending that escapes, not the whole of _CHECK_PERIODIC and we want the code generator to know that.

Copy link
Member

Choose a reason for hiding this comment

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

I meant a macro instruction of the DSL.

Copy link
Member Author

@markshannon markshannon Aug 14, 2024

Choose a reason for hiding this comment

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

Ah, I see.
No it can't, as macros can only be composed in a linear sequence. We need branching here, not deoptimization.
EXIT_IF and DEOPT_IF only make sense in specialized instructions.

@markshannon markshannon merged commit eec7bda into python:main Aug 14, 2024
65 checks passed
@markshannon markshannon deleted the eval-breaker-micro-ops branch August 14, 2024 11:04
blhsing pushed a commit to blhsing/cpython that referenced this pull request Aug 22, 2024
* Factor some instructions into micro-ops to isolate CHECK_EVAL_BREAKER for escape analysis

* Eliminate CHECK_EVAL_BREAKER macro
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