[PATCH] D50876: Clean up newly created header

mclow.lists closed this revision. mclow.lists added a comment. Landed as r340049 https://reviews.llvm.org/D50876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50876: Clean up newly created header

mclow.lists added a comment. In https://reviews.llvm.org/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.

[PATCH] D50876: Clean up newly created header

mclow.lists added a comment. @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. https://reviews.llvm.org/D50876 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D50876: Clean up newly created header

craig.topper added inline comments. Comment at: include/bit:140 static_assert(sizeof(unsigned) == 4, ""); return __popcnt(__x); } MSVC blindly uses the popcnt instruction whenever it sees this intrinsic. So this only works on Nehalem and newer Intel CPUs.

[PATCH] D50876: Clean up newly created header

mclow.lists added inline comments. Comment at: include/bit:96 #if defined(_LIBCPP_HAS_BITSCAN64) (defined(_M_AMD64) || defined(__x86_64__)) + if (_BitScanForward64(&__where, __x)) I'm not sure this code is ever used - since how can this compile? https://

[PATCH] D50876: Clean up newly created header

mclow.lists updated this revision to Diff 161284. mclow.lists added a comment. Clean up the windows code a bit - though I don't think is used - since I don't think it will compile. https://reviews.llvm.org/D50876 Files: include/__bit_reference include/bit Index: include/bit ==

[PATCH] D50876: Clean up newly created header

mclow.lists added inline comments. Comment at: include/bit:61 +inline _LIBCPP_INLINE_VISIBILITY +int __popcount(unsigned __x) { return __builtin_popcount (__x); } + ldionne wrote: > Funny spacing between `__builtin_popcount` and `(__x)` It's actually t

[PATCH] D50876: Clean up newly created header

mclow.lists added inline comments. Comment at: include/bit:117 + unsigned long __where; // Search from LSB to MSB for first set bit. // Returns zero if no set bit is found. lebedev.ri wrote: > Like i commented in the original review, this should probably b

[PATCH] D50876: Clean up newly created header

lebedev.ri added inline comments. Comment at: include/bit:117 + unsigned long __where; // Search from LSB to MSB for first set bit. // Returns zero if no set bit is found. Like i commented in the original review, this should probably be ``` // Search from

[PATCH] D50876: Clean up newly created header

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM with the `__clz` you missed. Comment at: include/bit:61 +inline _LIBCPP_INLINE_VISIBILITY +int __popcount(unsigned __x) { return __builtin_popcount (__x); }

[PATCH] D50876: Clean up newly created header

mclow.lists added inline comments. Comment at: include/bit:113 inline _LIBCPP_INLINE_VISIBILITY unsigned __clz(unsigned __x) { static_assert(sizeof(unsigned) == sizeof(unsigned long), ""); Missed this one. Should be `int` https://reviews.llvm.org/D50876

[PATCH] D50876: Clean up newly created header

mclow.lists created this revision. mclow.lists added reviewers: ldionne, EricWF. 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 ter