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-119180: Add LOAD_COMMON_CONSTANT opcode #119321

Merged
merged 7 commits into from
May 22, 2024

Conversation

JelleZijlstra
Copy link
Member

@JelleZijlstra JelleZijlstra commented May 21, 2024

The PEP 649 implementation will require a way to load NotImplementedError
from the bytecode. @markshannon suggested implementing this by converting
LOAD_ASSERTION_ERROR into a more general mechanism for loading constants.

This PR adds this new opcode. I will work on the rest of the implementation
of the PEP separately.


📚 Documentation preview 📚: https://cpython-previews--119321.org.readthedocs.build/

The PEP 649 implementation will require a way to load NotImplementedError
from the bytecode. @markshannon suggested implementing this by converting
LOAD_ASSERTION_ERROR into a more general mechanism for loading constants.

This PR adds this new opcode. I will work on the rest of the implementation
of the PEP separately.
Python/ceval_macros.h Outdated Show resolved Hide resolved
@JelleZijlstra
Copy link
Member Author

The previous version broke the JIT. @brandtbucher helped figure out that this is because the common_constants array is empty when the JIT is being compiled. Since there didn't seem to be a simple way to make this work correctly, we decided to simply use a switch in the interpreter code instead.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Here you go!

Include/internal/pycore_opcode_metadata.h Outdated Show resolved Hide resolved
Lib/importlib/_bootstrap_external.py Outdated Show resolved Hide resolved
Python/bytecodes.c Outdated Show resolved Hide resolved
Python/ceval.c Outdated Show resolved Hide resolved
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Yay!

@JelleZijlstra JelleZijlstra enabled auto-merge (squash) May 21, 2024 20:31
@JelleZijlstra JelleZijlstra merged commit 98e855f into python:main May 22, 2024
55 checks passed
@JelleZijlstra JelleZijlstra deleted the pep649-loadcommon branch May 22, 2024 12:26
JelleZijlstra added a commit that referenced this pull request May 25, 2024
PR #119321 added a comment about the magic number bump
but did not actually apply the new magic number.
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
The PEP 649 implementation will require a way to load NotImplementedError
from the bytecode. @markshannon suggested implementing this by converting
LOAD_ASSERTION_ERROR into a more general mechanism for loading constants.

This PR adds this new opcode. I will work on the rest of the implementation
of the PEP separately.

Co-authored-by: Irit Katriel <[email protected]>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
PR python#119321 added a comment about the magic number bump
but did not actually apply the new magic number.
@JelleZijlstra JelleZijlstra restored the pep649-loadcommon branch September 10, 2024 23:37
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.

4 participants