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-100982: Add `COMPARE_AND_BRANCH` instruction by markshannon · Pull Request #100983 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Jan 12, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: Specialization of some instructions does not conform to PEP 659, and prevents PEP 669 #100982

Sorry, something went wrong.

Uh oh!

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

All reactions
markshannon added 3 commits January 12, 2023 12:28
@markshannon
Add COMPARE_AND_BRANCH instruction
e5059b2
@markshannon
Update summarize stats script to handle both COMPAREs.
482777c
@markshannon
Remove attempted specialization of COMPARE_OP.
358393d
@markshannon
markshannon requested a review from iritkatriel as a code owner January 12, 2023 13:44
@bedevere-bot bedevere-bot added the awaiting core review label Jan 12, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jan 12, 2023
Specialization of some instructions does not conform to PEP 659, and prevents PEP 669 #100982
Closed
@markshannon
Add news
c5c1764
iritkatriel
iritkatriel reviewed Jan 12, 2023
View reviewed changes
Comment thread Lib/opcode.py
jrel_op('JUMP_BACKWARD', 140) # Number of words to skip (backwards)

def_op('COMPARE_AND_BRANCH', 141) # Comparison and jump
hascompare.append(141)

@iritkatriel iritkatriel Jan 12, 2023

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.

This opcode needs to be in hasjrel as well.

Sorry, something went wrong.

Uh oh!

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

All reactions

@markshannon markshannon Jan 12, 2023

Copy link
Copy Markdown
Member Author

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.

The problem is that dis sees COMPARE_AND_BRANCH, then POP_JUMP_IF_. The POP_JUMP_IF_ is where we jump from.
So marking COMPARE_AND_BRANCH as a jump confuses dis, it thinks the operator is a jump offset. COMPARE_AND_BRANCH is effectively a superinstruction.

I could special case COMPARE_AND_BRANCH in dis, but I think it might be better to wait for proper support for longer instructions.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Lib/importlib/_bootstrap_external.py Resolved

Uh oh!

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

markshannon added 4 commits January 12, 2023 15:44
@markshannon
Document COMPARE_OP instruction.
944147c
@markshannon
Document the correct instruction
ef3fe45
@markshannon
Regenerate file
447f63e
@markshannon
Revert change to COMPARE_AND_BRANCH classification.
7d33508
iritkatriel
iritkatriel approved these changes Jan 16, 2023
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jan 16, 2023
iritkatriel
iritkatriel reviewed Jan 16, 2023
View reviewed changes
Comment thread Python/specialize.c Resolved

Uh oh!

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

markshannon added 2 commits January 16, 2023 10:52
@markshannon
Avoid unused variable warning when NDEBUG is defined.
776ba54
@markshannon
Merge branch 'main' into compare-and-branch-instruction-2
5b01cc5
@markshannon
markshannon merged commit 7b14c2e into python:main Jan 16, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Jan 16, 2023
@oscarbenjamin oscarbenjamin mentioned this pull request Feb 26, 2023
Regression segfault under CPython 3.12a5 in the SymPy test suite #102250
Closed
@markshannon
markshannon deleted the compare-and-branch-instruction-2 branch September 26, 2023 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

3 participants

@markshannon @iritkatriel @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.