NotificationsYou must be signed in to change notification settings
Fork32.1k
Star67.4k

GH-100982: Add`COMPARE_AND_BRANCH` instruction#100983

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

markshannon merged 10 commits intopython:mainfromfaster-cpython:compare-and-branch-instruction-2

Jan 16, 2023

Merged

GH-100982: Add`COMPARE_AND_BRANCH` instruction#100983

markshannon merged 10 commits intopython:mainfromfaster-cpython:compare-and-branch-instruction-2

Jan 16, 2023

Conversation

Copy link

Member

markshannon commentedJan 12, 2023•
edited by bedevere-bot
Loading

Issue:Specialization of some instructions does not conform to PEP 659, and prevents PEP 669 #100982

markshannon added3 commits

January 12, 2023 12:28

Add COMPARE_AND_BRANCH instruction

e5059b2

Update summarize stats script to handle both COMPAREs.

482777c

Remove attempted specialization of COMPARE_OP.

358393d

markshannon requested a review fromiritkatriel as acode owner

January 12, 2023 13:44

bedevere-bot added the awaiting core review label

Jan 12, 2023

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

Add news

c5c1764

iritkatriel reviewed

Jan 12, 2023

View reviewed changes

Lib/opcode.py

		@@ -189,7 +189,8 @@ def pseudo_op(name, op, real_ops):
		def_op('DELETE_DEREF', 139)
		hasfree.append(139)
		jrel_op('JUMP_BACKWARD', 140) # Number of words to skip (backwards)

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

Copy link

Member

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.

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

The problem is thatdis seesCOMPARE_AND_BRANCH, thenPOP_JUMP_IF_. ThePOP_JUMP_IF_ is where we jump from.
So markingCOMPARE_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 caseCOMPARE_AND_BRANCH in dis, but I think it might be better to wait for proper support for longer instructions.