Skip to content

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

Merged
markshannon merged 12 commits intopython:mainfrom
faster-cpython:subscr-slice
Jun 27, 2022
Merged

GH-94163: Add BINARY_SLICE and STORE_SLICE instructions.#94168
markshannon merged 12 commits intopython:mainfrom
faster-cpython:subscr-slice

Conversation

@markshannon
Copy link
Copy Markdown
Member

@markshannon markshannon commented Jun 23, 2022

@markshannon
Copy link
Copy Markdown
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
Copy Markdown
Member

The dis doc update is missing.


.. opcode:: BINARY_SLICE

Implements ``TOS = TOS2[TOS1:TOS]``.
Copy link
Copy Markdown
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.

extern void _PySlice_Fini(PyInterpreterState *);

extern PyObject *
_PyBuildSlice_Consume2(PyObject *start, PyObject *stop);
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

🤖 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
Copy Markdown
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