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-104909: Split more LOAD_ATTR specializations by gvanrossum · Pull Request #110317 · python/cpython

Conversation

@gvanrossum

@gvanrossum gvanrossum commented Oct 3, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Split the following LOAD_ATTR specializations:

  • LOAD_ATTR_NONDESCRIPTOR_NO_DICT
  • LOAD_ATTR_METHOD_LAZY_DICT
  • LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES
  • LOAD_ATTR_CLASS
  • LOAD_ATTR_WITH_HINT
  • LOAD_ATTR_MODULE

This also contains a cosmetic fix regarding the indentation of DEOPT_IF() macros.

The following two are the only LOAD_ATTR specialization that aren't split up yet:

  • LOAD_ATTR_PROPERTY (looks complicated)
  • LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN (not used in the benchmark)
  • Issue: Split opcodes into micro-ops #104909

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
gvanrossum added 8 commits October 3, 2023 12:11
@gvanrossum
Split LOAD_ATTR_MODULE
416cc01
@gvanrossum
Split LOAD_ATTR_WITH_HINT
c96a6da
@gvanrossum
Split _GUARD_TYPE_VERSION out of the latter
de86f0c
@gvanrossum
Split LOAD_ATTR_CLASS
a45d861
@gvanrossum
Split LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES
74c917b
@gvanrossum
Fix indent of DEOPT_IF in macros
0f9fa7f
@gvanrossum
Split LOAD_ATTR_METHOD_LAZY_DICT
ea0e772
@gvanrossum
Split LOAD_ATTR_NONDESCRIPTOR_NO_DICT
b3e16a4
@bedevere-app bedevere-app Bot added the awaiting review label Oct 3, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Oct 3, 2023
Split opcodes into micro-ops #104909
Closed
@gvanrossum gvanrossum added the skip news label Oct 3, 2023
@gvanrossum

gvanrossum commented Oct 3, 2023

Copy link
Copy Markdown
Member Author

Oh. Test failures: test_bz2 and test_lzma. Bisecting.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gvanrossum added 2 commits October 3, 2023 16:04
@gvanrossum
Fix omission of _CHECK_ATTR_METHOD_LAZY_DICT
6e8718e
@gvanrossum
Merge remote-tracking branch 'upstream/main' into split-uops
8ab398d
@gvanrossum

gvanrossum commented Oct 4, 2023

Copy link
Copy Markdown
Member Author

I'm not sure I believe it, but the benchmarks say tthis is 1% faster:

Geometric mean: 1.01x faster (HPT: reliability of 99.88%, 1.00x faster at 99th %ile)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon markshannon mentioned this pull request Oct 4, 2023
Handle long tail of specialization for LOAD_ATTR and CALL with trampolines. faster-cpython/ideas#447
Open
@markshannon
markshannon self-requested a review October 4, 2023 15:06
@markshannon

markshannon commented Oct 4, 2023

Copy link
Copy Markdown
Member

Looks good.
Not too many instructions left to convert now.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@markshannon
markshannon merged commit 7c149a7 into python:main Oct 4, 2023
@bedevere-app bedevere-app Bot removed the awaiting review label Oct 4, 2023
@gvanrossum
gvanrossum deleted the split-uops branch October 4, 2023 15:19
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@gvanrossum @Glyphack
pythongh-104909: Split more LOAD_ATTR specializations (pythonGH-110317)
8cad4fb 
* Split LOAD_ATTR_MODULE

* Split LOAD_ATTR_WITH_HINT

* Split _GUARD_TYPE_VERSION out of the latter

* Split LOAD_ATTR_CLASS

* Split LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES

* Fix indent of DEOPT_IF in macros

* Split LOAD_ATTR_METHOD_LAZY_DICT

* Split LOAD_ATTR_NONDESCRIPTOR_NO_DICT

* Fix omission of _CHECK_ATTR_METHOD_LAZY_DICT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

skip news

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.

2 participants

@gvanrossum @markshannon
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 01402520 — 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.