Skip to content

Untag dispatch - #2660

Merged
Stephan T. Lavavej (StephanTLavavej) merged 5 commits into
microsoft:mainfrom
AlexGuteniev:untag_dispatch_string_find
May 1, 2022
Merged

Untag dispatch #2660
Stephan T. Lavavej (StephanTLavavej) merged 5 commits into
microsoft:mainfrom
AlexGuteniev:untag_dispatch_string_find

Conversation

@AlexGuteniev

@AlexGuteniev Alex Guteniev (AlexGuteniev) commented Apr 21, 2022

Copy link
Copy Markdown
Contributor

Towards #189

Same as #2479, nothing to see here

@AlexGuteniev
Untag dispatch
25c40ec 
Same as microsoft#2479, nothing to see here
@AlexGuteniev
Alex Guteniev (AlexGuteniev) requested a review from a team as a code owner April 21, 2022 17:16
@CaseyCarter Casey Carter (CaseyCarter) added the enhancement Something can be improved label Apr 21, 2022
@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) added throughput Must compile faster and removed enhancement Something can be improved labels Apr 21, 2022
@StephanTLavavej

This comment was marked as resolved.

Sign in to view
@AlexGuteniev

This comment was marked as resolved.

Sign in to view
Open
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Apr 28, 2022

Copy link
Copy Markdown
Member

Looks great, thanks for the continued if constexprization!

I pushed a trivial change to remove // general algorithm comments (matching your previous _Traits_find_first_of() change) - these were the counterparts of the // special case for std::char_traits comments, and both are no longer necessary now that there's a clear if constexpr (_Special) { ... } else { ... }.

Note to reviewers: as before, GitHub makes this diff look way worse than it really is. I copied each pair of old and new implementations into VS Code editors, then reordered the old overloads so that the "special" case occurs first. Along with a bit of indentation, this made the diff easy to read.

@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented Apr 30, 2022

Copy link
Copy Markdown
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Stephan T. Lavavej (StephanTLavavej) merged commit a10c471 into microsoft:main May 1, 2022
@StephanTLavavej

Stephan T. Lavavej (StephanTLavavej) commented May 1, 2022

Copy link
Copy Markdown
Member

Thanks for completing the untag dispatching of 's find functions and improving throughput! 🚀 🐇 ⏱️

@AlexGuteniev
Alex Guteniev (AlexGuteniev) deleted the untag_dispatch_string_find branch May 1, 2022 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej) StephanTLavavej approved these changes
+2 more reviewers
@CaseyCarter Casey Carter (CaseyCarter) CaseyCarter approved these changes
@raghavthind2005 Raghav Thind (raghavthind2005) raghavthind2005 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej Stephan T. Lavavej (StephanTLavavej)

Labels

throughput Must compile faster

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AlexGuteniev @StephanTLavavej @CaseyCarter @raghavthind2005