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-94163: Add BINARY_SLICE and STORE_SLICE instructions. by markshannon · Pull Request #94168 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Jun 23, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member
  • Issue: Add BINARY_SLICE and STORE_SLICE instructions. #94163

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
markshannon added 4 commits June 23, 2022 11:59
@markshannon
Handle slicing explicitly in the bytecode.
31e35e1
@markshannon
Merge branch 'main' into subscr-slice
aa095e5
@markshannon
Cleanup test.
6d01e79
@markshannon
Tidy up compiler a bit.
98ae795
@bedevere-bot bedevere-bot added the awaiting core review label Jun 23, 2022
markshannon added 3 commits June 23, 2022 13:43
@markshannon
Regen files.
4dec484
@markshannon
Mark BINARY_SLICE and STORE_SLICE as specializable for stats.
c8c569b
@markshannon
Avoid division by zero error in summarize stats script.
1c7dcde
@markshannon

markshannon commented Jun 23, 2022

Copy link
Copy Markdown
Member Author

Pyperformance shows a nominally slowdown, but in the noise.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
markshannon requested a review from iritkatriel June 23, 2022 15:18
@iritkatriel

iritkatriel commented Jun 23, 2022

Copy link
Copy Markdown
Member

The dis doc update is missing.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
Document new instructions.
be43b4c
iritkatriel
iritkatriel reviewed Jun 24, 2022
View reviewed changes
Comment thread Objects/sliceobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/library/dis.rst

.. opcode:: BINARY_SLICE

Implements ``TOS = TOS2[TOS1:TOS]``.

@iritkatriel iritkatriel Jun 24, 2022

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_sliceobject.h Outdated
extern void _PySlice_Fini(PyInterpreterState *);

extern PyObject *
_PyBuildSlice_Consume2(PyObject *start, PyObject *stop);

@iritkatriel iritkatriel Jun 24, 2022

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
kumaraditya303
kumaraditya303 reviewed Jun 24, 2022
View reviewed changes
Comment thread Misc/NEWS.d/next/Core and Builtins/2022-06-23-12-10-39.gh-issue-94163.SqAfQq.rst Outdated
@@ -0,0 +1,3 @@
Add BINARY_SLICE and STORE_SLICE instructions for more efficient handling

@kumaraditya303 kumaraditya303 Jun 24, 2022

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
markshannon added 4 commits June 24, 2022 15:22
@markshannon
Use rst formatting in news item.
96e8135
@markshannon
Refactor to remove duplicated code.
2ac4ba0
@markshannon
Explain what TOS1, TOS2 and TOS3 are.
05bff92
@markshannon
Update function name.
c1769e1
iritkatriel
iritkatriel approved these changes Jun 24, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jun 24, 2022
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 24, 2022
@bedevere-bot

bedevere-bot commented Jun 24, 2022

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 24, 2022
@markshannon

markshannon commented Jun 27, 2022

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
markshannon merged commit c0453a4 into python:main Jun 27, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Jun 27, 2022
@tekknolagi tekknolagi mentioned this pull request Jun 12, 2023
Rewrite BUILD_SLICE to LOAD_SLICE_CACHED when it's constant tekknolagi/skybison#464
Draft
@markshannon
markshannon deleted the subscr-slice branch September 26, 2023 12:49
@mart-e mart-e mentioned this pull request Dec 12, 2024
[FIX] safe_eval: add STORE_SLICE odoo/odoo#187867
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments
@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.

4 participants

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