Clean up newly created <bit> header

[mclow.lists]

Summary

Still NFC here.

1. Take the 9 functions that each had an #ifndef _LIBCPP_COMPILER_MSVC .. #else .. endif block in them and make just two blocks, each with 9 functions. Much easier to read, but makes for a terrible diff.

2. Change the return type of __ctz and __clz from the same type as the parameter to int. I would have preferred unsigned, but that's not what P0553 gave us. I reviewed all the call sites for these functions, and for __ctz it was always immediately static_cast-ed to another type, and __clz it made no difference either.

3. Change the name __pop_count to __popcount to match the name that we're going to add from P0553.

4. Rename the local variable in the windows code from where to __where. Shame on someone ;-)

[mclow.lists]

include/bit

106–107

Missed this one. Should be int

[ldionne]

LGTM with the __clz you missed.

include/bit

61

Funny spacing between __builtin_popcount and (__x)

[lebedev.ri]

include/bit

109–110

Like i commented in the original review, this should probably be

// Search from *M*SB to *L*SB for first set bit.

[mclow.lists]

include/bit

109–110

I think my preference would be to just remove the comment.

[mclow.lists]

include/bit

61

It's actually there to line up with the ones below.
(Though now that I look, I didn't do that for the ones above)

[mclow.lists]

Clean up the windows code a bit - though I don't think is used - since I don't think it will compile.

include/bit

92–93

I'm not sure this code is ever used - since how can this compile?

[craig.topper]

include/bit

140

MSVC blindly uses the popcnt instruction whenever it sees this intrinsic. So this only works on Nehalem and newer Intel CPUs. And Barcelona and newer AMD CPUs. This is why llvm uses a bit math version of popcnt for MSVC in include/llvm/Support/MathExtras.h

150

This doesn't exist in 32-bit MSVC.

[mclow.lists]

@craig.topper - that's existing code; I'm not changing it.
If we have a test bot that I can test this against, I'm happy to update it.

[mclow.lists]

In D50876#1204531, @mclow.lists wrote:

@craig.topper - that's existing code; I'm not changing it.
If we have a test bot that I can test this against, I'm happy to update it.

I'm not really sure that this code is actually used anywhere.

[mclow.lists]

Landed as r340049

include/__bit_reference

 #define _LIBCPP___BIT_REFERENCE
 
 #include <__config>
+#include <bit>
 #include <algorithm>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)

         __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
         __storage_type __dn = _VSTD::min(__clz_f, __n);
         __storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-        __r = _VSTD::__pop_count(*__first.__seg_ & __m);
+        __r = _VSTD::__popcount(*__first.__seg_ & __m);
         __n -= __dn;
         ++__first.__seg_;
     }
     // do middle whole words
     for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word)
-        __r += _VSTD::__pop_count(*__first.__seg_);
+        __r += _VSTD::__popcount(*__first.__seg_);
     // do last partial word
     if (__n > 0)
     {
         __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-        __r += _VSTD::__pop_count(*__first.__seg_ & __m);
+        __r += _VSTD::__popcount(*__first.__seg_ & __m);
     }
     return __r;
 }

         __storage_type __clz_f = static_cast<__storage_type>(__bits_per_word - __first.__ctz_);
         __storage_type __dn = _VSTD::min(__clz_f, __n);
         __storage_type __m = (~__storage_type(0) << __first.__ctz_) & (~__storage_type(0) >> (__clz_f - __dn));
-        __r = _VSTD::__pop_count(~*__first.__seg_ & __m);
+        __r = _VSTD::__popcount(~*__first.__seg_ & __m);
         __n -= __dn;
         ++__first.__seg_;
     }
     // do middle whole words
     for (; __n >= __bits_per_word; ++__first.__seg_, __n -= __bits_per_word)
-        __r += _VSTD::__pop_count(~*__first.__seg_);
+        __r += _VSTD::__popcount(~*__first.__seg_);
     // do last partial word
     if (__n > 0)
     {
         __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
-        __r += _VSTD::__pop_count(~*__first.__seg_ & __m);
+        __r += _VSTD::__popcount(~*__first.__seg_ & __m);
     }
     return __r;
 }

include/bit

 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
+#ifndef _LIBCPP_COMPILER_MSVC
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __ctz(unsigned __x)           { return __builtin_ctz(__x); }
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __ctz(unsigned long __x)      { return __builtin_ctzl(__x); }
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __ctz(unsigned long long __x) { return __builtin_ctzll(__x); }
+
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __clz(unsigned __x)           { return __builtin_clz(__x); }
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __clz(unsigned long __x)      { return __builtin_clzl(__x); }
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __clz(unsigned long long __x) { return __builtin_clzll(__x); }
+
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __popcount(unsigned __x)           { return __builtin_popcount(__x); }
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __popcount(unsigned long __x)      { return __builtin_popcountl(__x); }
+
+inline _LIBCPP_INLINE_VISIBILITY
+int __popcount(unsigned long long __x) { return __builtin_popcountll(__x); }
+
+#else  // _LIBCPP_COMPILER_MSVC
+
 // Precondition:  __x != 0
 inline _LIBCPP_INLINE_VISIBILITY
-unsigned __ctz(unsigned __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-    return static_cast<unsigned>(__builtin_ctz(__x));
-#else
+int __ctz(unsigned __x) {
   static_assert(sizeof(unsigned) == sizeof(unsigned long), "");
   static_assert(sizeof(unsigned long) == 4, "");
-  unsigned long where;
-  // Search from LSB to MSB for first set bit.
-  // Returns zero if no set bit is found.
-  if (_BitScanForward(&where, __x))
-    return where;
+  unsigned long __where;
+  if (_BitScanForward(&__where, __x))
+    return static_cast<int>(__where);
   return 32;
-#endif
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-unsigned long __ctz(unsigned long __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-    return static_cast<unsigned long>(__builtin_ctzl(__x));
-#else
+int __ctz(unsigned long __x) {
     static_assert(sizeof(unsigned long) == sizeof(unsigned), "");
     return __ctz(static_cast<unsigned>(__x));
-#endif
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-unsigned long long __ctz(unsigned long long __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-    return static_cast<unsigned long long>(__builtin_ctzll(__x));
-#else
-    unsigned long where;
-// Search from LSB to MSB for first set bit.
-// Returns zero if no set bit is found.
+int __ctz(unsigned long long __x) {
+    unsigned long __where;
 #if defined(_LIBCPP_HAS_BITSCAN64)
     (defined(_M_AMD64) || defined(__x86_64__))
-  if (_BitScanForward64(&where, __x))
-    return static_cast<int>(where);
+  if (_BitScanForward64(&__where, __x))
+    return static_cast<int>(__where);
 #else
   // Win32 doesn't have _BitScanForward64 so emulate it with two 32 bit calls.
-  // Scan the Low Word.
-  if (_BitScanForward(&where, static_cast<unsigned long>(__x)))
-    return where;
-  // Scan the High Word.
-  if (_BitScanForward(&where, static_cast<unsigned long>(__x >> 32)))
-    return where + 32; // Create a bit offset from the LSB.
+  if (_BitScanForward(&__where, static_cast<unsigned long>(__x)))
+    return static_cast<int>(__where);
+  if (_BitScanForward(&__where, static_cast<unsigned long>(__x >> 32)))
+    return static_cast<int>(__where + 32);
 #endif
   return 64;
-#endif // _LIBCPP_COMPILER_MSVC
 }
 
 // Precondition:  __x != 0
 inline _LIBCPP_INLINE_VISIBILITY
-unsigned __clz(unsigned __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-    return static_cast<unsigned>(__builtin_clz(__x));
-#else
+int __clz(unsigned __x) {
   static_assert(sizeof(unsigned) == sizeof(unsigned long), "");
   static_assert(sizeof(unsigned long) == 4, "");
-  unsigned long where;
-  // Search from LSB to MSB for first set bit.
-  // Returns zero if no set bit is found.
-  if (_BitScanReverse(&where, __x))
-    return 31 - where;
+  unsigned long __where;
+  if (_BitScanReverse(&__where, __x))
+    return static_cast<int>(31 - __where);
   return 32; // Undefined Behavior.
-#endif
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-unsigned long __clz(unsigned long __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-    return static_cast<unsigned long>(__builtin_clzl (__x));
-#else
+int __clz(unsigned long __x) {
     static_assert(sizeof(unsigned) == sizeof(unsigned long), "");
     return __clz(static_cast<unsigned>(__x));
-#endif
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-unsigned long long __clz(unsigned long long __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-    return static_cast<unsigned long long>(__builtin_clzll(__x));
-#else
-  unsigned long where;
-// BitScanReverse scans from MSB to LSB for first set bit.
-// Returns 0 if no set bit is found.
+int __clz(unsigned long long __x) {
+  unsigned long __where;
 #if defined(_LIBCPP_HAS_BITSCAN64)
-  if (_BitScanReverse64(&where, __x))
-    return static_cast<int>(63 - where);
+  if (_BitScanReverse64(&__where, __x))
+    return static_cast<int>(63 - __where);
 #else
-  // Scan the high 32 bits.
-  if (_BitScanReverse(&where, static_cast<unsigned long>(__x >> 32)))
-    return 63 - (where + 32); // Create a bit offset from the MSB.
-  // Scan the low 32 bits.
-  if (_BitScanReverse(&where, static_cast<unsigned long>(__x)))
-    return 63 - where;
+  // Win32 doesn't have _BitScanReverse64 so emulate it with two 32 bit calls.
+  if (_BitScanReverse(&__where, static_cast<unsigned long>(__x >> 32)))
+    return static_cast<int>(63 - (__where + 32));
+  if (_BitScanReverse(&__where, static_cast<unsigned long>(__x)))
+    return static_cast<int>(63 - __where);
 #endif
   return 64; // Undefined Behavior.
-#endif // _LIBCPP_COMPILER_MSVC
 }
 
-inline _LIBCPP_INLINE_VISIBILITY int __pop_count(unsigned __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-  return __builtin_popcount  (__x);
-#else
+inline _LIBCPP_INLINE_VISIBILITY int __popcount(unsigned __x) {
   static_assert(sizeof(unsigned) == 4, "");
   return __popcnt(__x);
-#endif
 }
 
-inline _LIBCPP_INLINE_VISIBILITY int __pop_count(unsigned long __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-  return __builtin_popcountl (__x);
-#else
+inline _LIBCPP_INLINE_VISIBILITY int __popcount(unsigned long __x) {
   static_assert(sizeof(unsigned long) == 4, "");
   return __popcnt(__x);
-#endif
 }
 
-inline _LIBCPP_INLINE_VISIBILITY int __pop_count(unsigned long long __x) {
-#ifndef _LIBCPP_COMPILER_MSVC
-  return __builtin_popcountll(__x);
-#else
+inline _LIBCPP_INLINE_VISIBILITY int __popcount(unsigned long long __x) {
   static_assert(sizeof(unsigned long long) == 8, "");
   return __popcnt64(__x);
-#endif
 }
 
+#endif // _LIBCPP_COMPILER_MSVC
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP_BIT