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

bpo-43760: Streamline dispatch sequence for machines without computed gotos. #25244

Merged
merged 3 commits into from
Apr 8, 2021

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Apr 7, 2021

  • Do fetch and decode at end of opocde then jump directly to switch.
    Should allow compilers that don't support computed-gotos, specifically MSVC,
    to generate better code.

  • Cleans up the code

This should have no effect when computed gotos are used.

https://bugs.python.org/issue43760

@markshannon
Copy link
Member Author

Skipping NEWS, as this shouldn't have any noticeable effect. I'll add a NEWS item to the other PR for https://bugs.python.org/issue43760 as that should have a (just?) measurable effect.

@markshannon markshannon force-pushed the dispatch-refactor branch 2 times, most recently from feffa76 to d90907f Compare April 7, 2021 11:44
* Do fetch and decode at end of opocde then jump directly to switch.
  Should allow compilers that don't support computed-gotos, specifically MSVC,
  to generate better code.
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 7, 2021
@bedevere-bot
Copy link

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

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 Apr 7, 2021
… function changed frame *and* raised an exception.
@rhettinger
Copy link
Contributor

Thanks for doing this. It looks like a nice improvement.

Why did you switch to uint8_t? That doesn't seem consistent with the rest of the code.

@markshannon
Copy link
Member Author

@rhettinger
The primary reason for the change to uint8_t will be in later PR. If we add cases for all unknown opcodes then the compiler will know that the cases are exhaustive and won't need to emit bounds checks when dispatching.
I'll revert it to int for now.

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

Successfully merging this pull request may close these issues.

4 participants