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-105481: generate _specializations and _specialized_instructions from bytecodes.c #105913

Merged
merged 9 commits into from
Jun 19, 2023

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jun 19, 2023

This generates the metadata about specialised opcodes from bytecodes.c, to replace the hard coded dict in opcode.py.

There is one irregularity that I needed to special-case: BINARY_OP_INPLACE_ADD_UNICODE, which is commented out in bytecodes.c (because it has different stack effect compared to the rest of the family).

@iritkatriel iritkatriel requested a review from a team as a code owner June 19, 2023 14:01
@markshannon
Copy link
Member

markshannon commented Jun 19, 2023

We should move the optimization in BINARY_OP_INPLACE_ADD_UNICODE to tier 2, once it we have a tier 2 optimizer. It makes sense there, but not in tier 1.
For now we can just remove it. I suspect BINARY_OP_INPLACE_ADD_UNICODE is not safe for PEP 669 anyway.
#100982

Lib/importlib/_bootstrap_external.py Show resolved Hide resolved
Lib/test/test__opcode.py Outdated Show resolved Hide resolved
@markshannon
Copy link
Member

Looks like summarize_stats.py needs updating

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.

LGTM. One nit; no need to get another review if you decide to fix that. Also okay to merge as-is.

Makefile.pre.in Outdated Show resolved Hide resolved
@iritkatriel iritkatriel merged commit 33f0a85 into python:main Jun 19, 2023
@iritkatriel iritkatriel deleted the specialziation-data branch June 19, 2023 22:47
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.

4 participants