Skip to content

GH-94163: Add BINARY_SLICE and STORE_SLICE instructions. #94168

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

Merged
merged 12 commits into from
Jun 27, 2022

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Jun 23, 2022

@markshannon
Copy link
Member Author

Pyperformance shows a nominally slowdown, but in the noise.

@markshannon markshannon requested a review from iritkatriel June 23, 2022 15:18
@iritkatriel
Copy link
Member

The dis doc update is missing.

@@ -479,6 +479,20 @@ the original TOS1.
Implements ``del TOS1[TOS]``.


.. opcode:: BINARY_SLICE

Implements ``TOS = TOS2[TOS1:TOS]``.
Copy link
Member

Choose a reason for hiding this comment

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

I think the preamble to this section in lines 449-456 needs to be updated because it only refers to TOS and TOS1.

@@ -13,6 +13,8 @@ extern "C" {

extern void _PySlice_Fini(PyInterpreterState *);

extern PyObject *
_PyBuildSlice_Consume2(PyObject *start, PyObject *stop);
Copy link
Member

Choose a reason for hiding this comment

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

Consume here refers to references of start and stop? Can it be more explicit, maybe _PyBuildSlice_ConsumeRefs?

@@ -0,0 +1,3 @@
Add BINARY_SLICE and STORE_SLICE instructions for more efficient handling
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Add BINARY_SLICE and STORE_SLICE instructions for more efficient handling
Add :opcode:`BINARY_SLICE` and :opcode:`STORE_SLICE` instructions for more efficient handling

@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 24, 2022
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @markshannon for commit c1769e1 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 24, 2022
@markshannon
Copy link
Member Author

Failures on the AMD64 FreeBSD buildbot don't seem to have anything to do with this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants