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

Untag dispatch find #2380

Merged
merged 27 commits into from
Jan 6, 2022
Merged

Conversation

AlexGuteniev
Copy link
Contributor

Towards #189

Extracted from #2219

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner December 5, 2021 17:39
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Dec 6, 2021
@StephanTLavavej StephanTLavavej added throughput Must compile faster and removed enhancement Something can be improved labels Dec 7, 2021
stl/inc/xutility Outdated Show resolved Hide resolved
Comment on lines +5144 to +5146
if (!_Within_limits(_First, _Val)) {
return _Last;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: I observe that originally, in C++20 when is_constant_evaluated() was true, we would perform the _Within_limits check and early return if possible. Now, when is_constant_evaluated() is true, we simply activate the classic algorithm fallback. This seems fine, since at most there would be a minor throughput improvement, and it seems fairly unusual for the _Within_limits check to actually be successful (when it isn't, we pay the cost of a branch).

That is, _Within_limits is necessary for the runtime memchr optimization, but it's not really valuable for the constexpr case, and the new code is organized accordingly. 😸

stl/inc/xutility Outdated Show resolved Hide resolved
Co-authored-by: S. B. Tam <[email protected]>
stl/inc/xutility Show resolved Hide resolved
@CaseyCarter CaseyCarter self-assigned this Jan 6, 2022
@CaseyCarter
Copy link
Member

I'm going to add this to the next batch of changes to merge - please notify me if any further commits are pushed.

@CaseyCarter CaseyCarter merged commit 69541fa into microsoft:main Jan 6, 2022
@CaseyCarter CaseyCarter removed their assignment Jan 6, 2022
@CaseyCarter
Copy link
Member

Thanks for finding another place in the codebase to modernize! :cat-emoji: :more-cat-emoji:

@AlexGuteniev AlexGuteniev deleted the untag_dispatch_find branch January 9, 2022 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
throughput Must compile faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants