GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-105481: generate _specializations and _specialized_instructions from bytecodes.c by iritkatriel · Pull Request #105913 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Jun 19, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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).

  • Issue: Generate opcode metadata from bytecodes.c instead of opcode.py #105481

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner and gvanrossum reacted with thumbs up emoji Fidget-Spinner and Eclips4 reacted with rocket emoji
All reactions
  • 👍 2 reactions
  • 🚀 2 reactions
@iritkatriel
generate _specializations data from bytecodes.c
4096133
@iritkatriel
iritkatriel requested review from brettcannon, ericsnowcurrently, ncoghlan and warsaw as code owners June 19, 2023 13:27
@bedevere-bot bedevere-bot added the awaiting core review label Jun 19, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jun 19, 2023
Generate opcode metadata from bytecodes.c instead of opcode.py #105481
Closed
@iritkatriel
iritkatriel requested a review from gvanrossum June 19, 2023 13:28
@iritkatriel iritkatriel added the skip news label Jun 19, 2023
@iritkatriel
update PCbuild/regen.targets
66314d9
@iritkatriel
iritkatriel requested a review from a team as a code owner June 19, 2023 14:01
@iritkatriel
update a couple of tests
19aa0c7
@markshannon

markshannon commented Jun 19, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
markshannon
markshannon approved these changes Jun 19, 2023
View reviewed changes
Comment thread Lib/importlib/_bootstrap_external.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/test__opcode.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jun 19, 2023
@markshannon

markshannon commented Jun 19, 2023

Copy link
Copy Markdown
Member

Looks like summarize_stats.py needs updating
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel added 4 commits June 19, 2023 16:52
@iritkatriel
include stuff from _opcode_metadata in opcode
b7ae5cd
@iritkatriel
update import
d8fac92
@iritkatriel
tolerate _opcode_metadata not being available in early stages of the …
854ddf3 
…build
@iritkatriel
fix case
7ed6716
gvanrossum
gvanrossum approved these changes Jun 19, 2023
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Makefile.pre.in Resolved

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel added 2 commits June 19, 2023 21:50
@iritkatriel
reorder args
4556c5e
@iritkatriel
typo
d9d3b80
@iritkatriel
iritkatriel merged commit 33f0a85 into python:main Jun 19, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label 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

Reviewers

@markshannon markshannon markshannon approved these changes
@gvanrossum gvanrossum gvanrossum approved these changes
@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@iritkatriel @markshannon @gvanrossum @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.