Skip to content

gh-119180: Add LOAD_COMMON_CONSTANT opcode#119321

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.

Sign up for GitHub

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

Jump to bottom
Merged
JelleZijlstra merged 7 commits into from
May 22, 2024
Merged

gh-119180: Add LOAD_COMMON_CONSTANT opcode #119321

JelleZijlstra merged 7 commits into from
May 22, 2024

Conversation

@JelleZijlstra
Copy link
Member

@JelleZijlstraJelleZijlstra commented May 21, 2024•
edited by github-actions bot
Loading

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/

@JelleZijlstra
pythongh-119180: Add LOAD_COMMON_CONSTANT opcode
00d6eeb
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.
@bedevere-appbedevere-appbot mentioned this pull request May 21, 2024
Closed
30 tasks
JelleZijlstra
JelleZijlstra commented May 21, 2024
View reviewed changes
iritkatriel
iritkatriel reviewed May 21, 2024
View reviewed changes
JelleZijlstraand others added 3 commits May 21, 2024 13:10
@JelleZijlstra@iritkatriel
Update Misc/NEWS.d/next/Core and Builtins/2024-05-21-12-17-02.gh-issu…
ab560cd
…e-119180.UNDUb9.rst Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@JelleZijlstra
Different way that should work on Windows
4d42dcc
@JelleZijlstra
Use a switch
32a2a73
@JelleZijlstra
Copy link
MemberAuthor

JelleZijlstra commented May 21, 2024

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.

carljm
carljm approved these changes May 21, 2024
View reviewed changes
gvanrossum
gvanrossum reviewed May 21, 2024
View reviewed changes
Copy link
Member

@gvanrossumgvanrossum 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!

gvanrossum
gvanrossum approved these changes May 21, 2024
View reviewed changes
Copy link
Member

@gvanrossumgvanrossum left a comment

Choose a reason for hiding this comment

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

Yay!

@JelleZijlstraJelleZijlstra enabled auto-merge (squash) May 21, 2024 20:31
@JelleZijlstraJelleZijlstra merged commit 98e855f into python:mainMay 22, 2024
@JelleZijlstraJelleZijlstra deleted the pep649-loadcommon branch May 22, 2024 12:26
@JelleZijlstraJelleZijlstra mentioned this pull request May 22, 2024
Merged
JelleZijlstra added a commit that referenced this pull request May 25, 2024
@JelleZijlstra
gh-119180: Update the magic number (#119397)
84be524
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
@JelleZijlstra@iritkatriel@estyxx
pythongh-119180: Add LOAD_COMMON_CONSTANT opcode (python#119321)
344a053
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 <1055913+iritkatriel@users.noreply.github.com>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@JelleZijlstra@estyxx
pythongh-119180: Update the magic number (python#119397)
1c6cb41
PR python#119321 added a comment about the magic number bump but did not actually apply the new magic number.
@JelleZijlstraJelleZijlstra restored the pep649-loadcommon branch September 10, 2024 23:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatrieliritkatrieliritkatriel left review comments

@carljmcarljmcarljm approved these changes

@gvanrossumgvanrossumgvanrossum approved these changes

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@brettcannonbrettcannonAwaiting requested review from brettcannonbrettcannon is a code owner

@ncoghlanncoghlanAwaiting requested review from ncoghlanncoghlan is a code owner

@warsawwarsawAwaiting requested review from warsawwarsaw is a code owner

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JelleZijlstra@carljm@iritkatriel@gvanrossum