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-37774: use Py_LIKELY/Py_UNLIKELY for vectorcall #15144

Open
wants to merge 3 commits into
base: master
from

Conversation

@jdemeyer
Copy link
Contributor

commented Aug 6, 2019

@@ -102,4 +102,15 @@

#define Py_UNREACHABLE() abort()


/* If we're using GCC, use __builtin_expect() */
#if defined(__GNUC__) && (__GNUC__ > 2)

This comment has been minimized.

Copy link
@methane

methane Aug 6, 2019

Member

I think __GNUC__ > 2 is redundant.

This comment has been minimized.

Copy link
@jdemeyer

jdemeyer Aug 6, 2019

Author Contributor

Redundant because Python won't compile with GCC 2.x anyway?

This comment has been minimized.

Copy link
@methane

methane Aug 7, 2019

Member

Because:

  • I don't know GCC 2.x supports all of C99 features we use in PEP 7.
  • I don't know how to use GCC 2 to compile Python in recent days.
  • I don't know who try to compile Python with GCC 2.

This comment has been minimized.

Copy link
@methane

methane Aug 7, 2019

Member

I found this page: https://gcc.gnu.org/c99status.html
We use some GCC>=3.0 features already.

  • designated initializers
  • new block scopes for selection and iteration statements
  • stdbool.h (GCC 2.95 had <stdbool.h>, but based on an early draft with major differences from C99 semantics.)

@methane methane added the skip news label Aug 7, 2019

@methane

methane approved these changes Aug 7, 2019

@pablogsal
Copy link
Member

left a comment

I am not super fan of exposing these macros, by I agree with @methane in that they are battle tested. Do we have some benchmarks numbers for the effect of this (with PGO, as PGO also activates branch-prediction annotations)?

@methane

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

I don't think these macros are useful for PGO build. In case of obmalloc.c, it only affects non-PGO build.

@jdemeyer jdemeyer force-pushed the jdemeyer:vectorcall_likely branch from 140acb8 to 7881bc9 Aug 21, 2019

@jdemeyer

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2019

Rebased to latest master, fixed conflicts in obmalloc.c

@markshannon: what's your opinion on this? It achieves the same speedup as #14735 but in a more structural way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.