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-104909: Split more LOAD_ATTR specializations #110317

Merged
merged 10 commits into from
Oct 4, 2023

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Oct 3, 2023

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)

@gvanrossum
Copy link
Member Author

Oh. Test failures: test_bz2 and test_lzma. Bisecting.

@gvanrossum
Copy link
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)

@markshannon
Copy link
Member

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

@markshannon markshannon merged commit 7c149a7 into python:main 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
* 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants