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-100239: specialize left and right shift ops on ints #129431

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

eendebakpt
Copy link
Contributor

@eendebakpt eendebakpt commented Jan 29, 2025

We add a specialization for >> and >> on compact integers.

  • For both lshift and rshift we have to avoid undefined behavior for shifts larger than the number of bits in a C long.
  • The lshift operator << can overflow, so we restrict shifts to values smaller than 17. Under this assumption the value of a << b with b <= 16) fit into a 32-bit or 64-bit integer depending on the platform.

Benchmark:

bench_lshift_compactint: Mean +- std dev: [mainx2] 381 us +- 33 us -> [prx2] 334 us +- 46 us: 1.14x faster
bench_rshift_compactint: Mean +- std dev: [mainx2] 398 us +- 34 us -> [prx2] 281 us +- 46 us: 1.41x faster

Benchmark hidden because not significant (1): bench_rshift_largeint

Geometric mean: 1.17x **faster**

(non-PGO, Windows)

There is a gain with the specialization, but on the other hand: how many shift operations on compact ints are there? For example the shifts are used in the uuid module, but always on large ints.

@eendebakpt eendebakpt added this to the hhhhhhhhhhhhhhhhhhhhhhhhhbbbbbbbb milestone Jan 29, 2025
@bswck
Copy link
Contributor

bswck commented Jan 29, 2025

@eendebakpt, something happened with the milestone.
image

@skirpichev skirpichev removed this from the hhhhhhhhhhhhhhhhhhhhhhhhhbbbbbbbb milestone Jan 29, 2025
@eendebakpt eendebakpt marked this pull request as draft January 29, 2025 19:51
@eendebakpt eendebakpt changed the title gh-100239: specialize right shift ops on ints gh-100239: specialize left and right shift ops on ints Jan 29, 2025
@eendebakpt eendebakpt marked this pull request as ready for review January 30, 2025 20:16
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.

3 participants