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

Optimize CeilLg2 to O(1) #139

Closed
wants to merge 1 commit into from
Closed

Conversation

OIer1048576
Copy link

No description provided.

@TumoiYorozu
Copy link
Contributor

__builtin_expect is wrong and cannot compile in either GCC or Clang.

Wrong:

if (__builtin_expect(n != 0)) return 32 - __builtin_clz(n - 1);

Collect:

if (__builtin_expect(n != 0, 1)) return 32 - __builtin_clz(n - 1);

Also, as a fundamental issue, __builtin_expect is not supported by Visual Studio, so it is better not to use __builtin_expect .

@yosupo06
Copy link
Collaborator

yosupo06 commented Mar 6, 2023

Thanks,

the reason why we didn't optimize ceil_pow2 is, the cost of this function willn't be any bottleneck in current usage. (In contrast, we call bsf in the convolution function many times so we have to optimize this)

As already you discussed, __builtin_xxx is a non-standard function of C++ and we want to avoid to use as much as possible.

related: #153

@yosupo06 yosupo06 closed this Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants