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-94163: Add BINARY_SLICE and STORE_SLICE instructions. #94168

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

@markshannon markshannon commented Jun 23, 2022

Pyperformance shows a nominally slowdown, but in the noise.

@markshannon markshannon requested a review from iritkatriel Jun 23, 2022
@iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jun 23, 2022

The dis doc update is missing.

Objects/sliceobject.c Show resolved Hide resolved
@@ -479,6 +479,20 @@ the original TOS1.
Implements ``del TOS1[TOS]``.


.. opcode:: BINARY_SLICE

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

@iritkatriel iritkatriel Jun 24, 2022

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

@iritkatriel iritkatriel Jun 24, 2022

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

@kumaraditya303 kumaraditya303 Jun 24, 2022

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 label Jun 24, 2022
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Jun 24, 2022

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

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots label Jun 24, 2022
@markshannon
Copy link
Member Author

@markshannon markshannon commented Jun 27, 2022

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

@markshannon markshannon merged commit c0453a4 into python:main Jun 27, 2022
83 of 86 checks passed
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.

None yet

4 participants