Skip to content

Conversation

@StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Nov 1, 2020

Fixes MS-internal VSO-1240399 / AB#1240399 reported by @xiangfan-ms.

  • <random>
    • Cleanup: change _Uty(MEOW), which is a functional-syntax C-semantics cast, to static_cast<_Uty>(MEOW).
    • Replace is_signed tag dispatch with if _CONSTEXPR_IF is_signed_v. This improves throughput twice (first because is_signed_v is silently optimized by MSVC, second because if constexpr is much cheaper than tag dispatch). Note that even when if constexpr is unavailable, both sides of the branch will compile just fine.
  • <xmemory>
    • Instead of "smashing" is_empty to bool_constant via typename MEOW::type, wrap is_empty_v in bool_constant because MSVC silently optimizes is_empty_v.
  • <xtree>
    • Similarly change this tag to bool_constant<is_same_v<MEOW>>.
  • <xutility>
    • Here, we were directly dispatching with is_signed converted to true_type or false_type during overload resolution. Again, it's faster to say bool_constant<is_signed_v<MEOW>>. (This logic should be refactored into if constexpr later, and/or it should use the C++20 in_range technique; I am deliberately not pursuing those larger changes now.)

@StephanTLavavej StephanTLavavej added the throughput Must compile faster label Nov 1, 2020
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner November 1, 2020 07:24
@StephanTLavavej

This comment has been minimized.

@azure-pipelines

This comment has been minimized.

@CaseyCarter CaseyCarter self-assigned this Nov 4, 2020
@CaseyCarter CaseyCarter removed their assignment Nov 4, 2020
StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Nov 5, 2020
StephanTLavavej added a commit to StephanTLavavej/STL that referenced this pull request Nov 5, 2020
template <class _InIt, class _Ty>
_NODISCARD constexpr bool _Within_limits(_InIt, const _Ty& _Val) { // check whether _Val is within the limits of _Elem
using _Elem = remove_pointer_t<_InIt>;
return _Within_limits(_Val, is_signed<_Elem>{}, is_signed<_Ty>{}, bool_constant<-1 == static_cast<_Ty>(-1)>{});
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this change as much as the others, I think it impacts clarity more. I'm OK with it if the throughput improvement is actually decent though.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was enough to show up on @xiangfan-ms's radar. Clarity will be dramatically improved when we can if constexprize this, hopefully soon.

@StephanTLavavej StephanTLavavej merged commit 7e07ed8 into microsoft:master Nov 7, 2020
@StephanTLavavej StephanTLavavej deleted the type_traits branch November 7, 2020 00:31
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.

3 participants