[patch] [libcxx] [win32] support.h

G M gmisocpp at gmail.com
Mon Dec 23 04:09:36 PST 2013


Hello Win32 libcxx watchers.
support.h implements the functions clz, clzl, clzll, ctz, ctzl, and ctzll
functions (I hate those names) for Win32 when the compiler does not support
them. So ms's cl.exe uses the implementations in support.h but clang and
gcc compilers provide these implementations themselves, not support.h.

Currently these support.h implementations have a few problems.

With the removal of windows.h from support.h they no longer compile. Even
when they did compile, they were incorrect at runtime. In more detail:

1. support.h now fails to compile for msvc (cl.exe) because it depended on
things like DWORD from windows.h which was removed in a prior commit and no
lighter weight replacement for windows.h was provided. This patch supplies
that with wtypes.h

2. These functions aren't implemented for 32 bit targets of cl.exe but they
should be as they are available in the 32 bit and 64 bit gcc and clang
targets. This is because the current implementations use BitScanForward64
etc. which aren't available to 32 bit targets. This patch provides 32 bit
implementations of the clt/l/ll and clz/l/ll functions to fix that
.
3. The above named functions current implementation in support.h is
incorrect. The return values of the support.h versions don't match the gcc
or clang return values when given the same inputs.

The attached support.h.diff patch fixes these problems.

Anyone wanting to verify the above statements: compile the attached test
case bittests.cpp with MS's 32 bit compiler cl.exe against the SVN version
of libcxx and it's support.h (i.e. before applying the support.h.diff
patch), you will see:
cl bittest.cpp -I/libcxx/include
c:\libcxx\include\support/win32/support.h(85) : error C2065: 'DWORD' :
undeclared identifier
<etc...> This is demonstrates problem 1 above.

If you edit that svn version of support.h and add wtypes.h and compile
again, you will see the first errors vanish. but then you then you will see
these errors:
c:\libcxx\include\support\win32\support.h(97) : error C3861:
'_BitScanReverse64': identifier not found
c:\libcxx\include\support\win32\support.h(112) : error C3861:
'_BitScanForward64': identifier not found
This demonstrates problem 2 above.

Switching to the 64 bit cl.exe compiler, if you compile with that, you
should be able to run it but get these runtime errors:
Test ctz failed. line 23, expected value: 32, actual value: 0
Test ctzl failed. line 33, expected value: 32, actual value: 0
Test ctzll failed. line 44, expected value: 64, actual value: 0
Test clz failed. line 57, expected value: 32, actual value: 0
<snip>
134 tests failed
This demonstrates problem number 3.

If you try compiling and runing this file with g++ or clang, you should see
no errors. This shows gcc and clang's implementations of the clt/clz
functions are ok yet support.h's version are not.
If you delete support.h and re get (assuming you modified it to add
wtypes.h) and then apply the support.h.diff patch and then re-compile with
g++, clang++ or cl.exe in 32 bit or 64 bit mode, the test case should then
run with no errors or output for all of those compilers, hopefully
indicating success with no regressions and proving that all the issues
stated were real and that this patch has fixed all the stated problems.

I am going on holiday the day after tomorrow and won't have access to email
unfortunately so I won't be able to reply to any questions after tomorrow
for about a week, but I thought I'd make this patch available in the mean
time for review and anyone who wants to play with it. I'll try to reply to
any early responses that come in to this patch before I go though.

Thanks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131224/baf8ccf3/attachment.html>
-------------- next part --------------
Index: support.h
===================================================================
--- support.h	(revision 197905)
+++ support.h	(working copy)
@@ -17,6 +17,14 @@
 
 #include <wchar.h>  // mbstate_t
 #include <cstdarg> // va_ macros
+// DWORD etc. required to implement builtins etc. for Win32
+// Not required for clang or gcc since the compilers provide those in that case.
+#if defined(_WIN32) && ! defined(__clang__) && ! defined(__GNUC__)
+#include <wtypes.h>
+#endif
+#if defined(_LIBCPP_MSVCRT)
+#include <xlocinfo.h>
+#endif
 #define swprintf _snwprintf
 #define vswprintf _vsnwprintf
 
@@ -36,7 +44,6 @@
 
 #if defined(_LIBCPP_MSVCRT)
 #define snprintf _snprintf
-#include <xlocinfo.h>
 #define atoll _atoi64
 #define strtoll _strtoi64
 #define strtoull _strtoui64
@@ -50,10 +57,9 @@
 { return _Stold(nptr, endptr, 0); }
 
 #define _Exit _exit
+#endif
 
-#ifndef __clang__ // MSVC-based Clang also defines _MSC_VER
-#include <intrin.h>
-
+#if ! defined(__clang__) && ! defined(__GNUC__) // CLANG and GCC have these already.
 _LIBCPP_ALWAYS_INLINE int __builtin_popcount(unsigned int x) {
    static const unsigned int m1 = 0x55555555; //binary: 0101...
    static const unsigned int m2 = 0x33333333; //binary: 00110011..
@@ -80,39 +86,101 @@
    return static_cast<int>((x * h01)>>56);  //returns left 8 bits of x + (x<<8) + (x<<16) + (x<<24) + ...
 }
 
-_LIBCPP_ALWAYS_INLINE int __builtin_ctz( unsigned int x )
+// Returns the number of trailing 0-bits in x,
+// starting at the least significant bit position.
+// If x is 0, the result is undefined. 
+_LIBCPP_ALWAYS_INLINE int __builtin_ctzll( unsigned long long mask )
 {
-   DWORD r = 0;
-   _BitScanReverse(&r, x);
-   return static_cast<int>(r);
+    DWORD where;
+    static_assert(sizeof(mask)==8,"");
+    static_assert(sizeof(DWORD)==4,"");
+    static_assert(sizeof(unsigned long)==4,"");
+    // Search from LSB to MSB for first set bit.
+    // Returns zero if no set bit is found.
+#if defined(_WIN64)
+    if (_BitScanForward64(&where, mask))
+        return static_cast<int>(where);
+#elif defined(_WIN32)
+    // Win32 doesn't have _BitScanForward64 so emulate it with two 32 bit calls. 
+    // Scan the Low Word
+    if (_BitScanForward(&where, static_cast<unsigned long>(mask)))
+        return static_cast<int>(where);
+    // Scan the High Word
+    if (_BitScanForward(&where, static_cast<unsigned long>(mask >> 32)))
+        return static_cast<int>(where+32); // Create a bit offset from the LSB.
+#else
+#   error "Implementation of __builtin_ctzll required"
+#endif
+    return 64;
 }
 
-// sizeof(long) == sizeof(int) on Windows
-_LIBCPP_ALWAYS_INLINE int __builtin_ctzl( unsigned long x )
-{ return __builtin_ctz( static_cast<int>(x) ); }
+_LIBCPP_ALWAYS_INLINE int __builtin_ctzl( unsigned long mask )
+{
+    // Win32 and Win64 expectations.
+    static_assert(sizeof(mask)==4,"");
+    static_assert(sizeof(DWORD)==4,"");
+    DWORD where;
+    // Search from LSB to MSB for first set bit.
+    // Returns zero if no set bit is found.
+    if (_BitScanForward(&where, mask))
+        return static_cast<int>(where);
+    return 32;
+}
 
-_LIBCPP_ALWAYS_INLINE int __builtin_ctzll( unsigned long long x )
+_LIBCPP_ALWAYS_INLINE int __builtin_ctz( unsigned int mask )
 {
-    DWORD r = 0;
-    _BitScanReverse64(&r, x);
-    return static_cast<int>(r);
+    // Win32 and Win64 expectations.
+    static_assert(sizeof(mask)==4,"");
+    static_assert(sizeof(unsigned long)==4,"");
+    return __builtin_ctzl(static_cast<unsigned long>(mask));
 }
-_LIBCPP_ALWAYS_INLINE int __builtin_clz( unsigned int x )
+
+// Returns the number of leading 0-bits in x,
+// starting at the most significant bit position.
+// If x is 0, the result is undefined.
+_LIBCPP_ALWAYS_INLINE int __builtin_clzll( unsigned long long mask )
 {
-   DWORD r = 0;
-   _BitScanForward(&r, x);
-   return static_cast<int>(r);
+    // Win32 and Win64 expectations.
+    static_assert(sizeof(mask)==8,"");
+    static_assert(sizeof(DWORD)==4,"");
+    DWORD where;
+    // BitScanReverse scans from MSB to LSB for first set bit.
+    // Returns 0 if no set bit is found.
+#if defined(_WIN64)
+    if (_BitScanReverse64(&where, mask))
+        return static_cast<int>(63-where);
+#elif defined(_WIN32)
+    // Scan the high 32 bits.
+    if (_BitScanReverse(&where, static_cast<DWORD>(mask >> 32)))
+        return static_cast<int>(63-(where+32)); // Create a bit offset from the MSB.
+    // Scan the low 32 bits.
+    if (_BitScanReverse(&where, static_cast<DWORD>(mask)))
+        return static_cast<int>(63-where);
+#else
+#   error "Implementation of __builtin_clzll required"
+#endif
+    return 64; // Undefined Behavior.
 }
-// sizeof(long) == sizeof(int) on Windows
-_LIBCPP_ALWAYS_INLINE int __builtin_clzl( unsigned long x )
-{ return __builtin_clz( static_cast<int>(x) ); }
-_LIBCPP_ALWAYS_INLINE int __builtin_clzll( unsigned long long x )
+
+_LIBCPP_ALWAYS_INLINE int __builtin_clzl( unsigned long mask )
 {
-    DWORD r = 0;
-    _BitScanForward64(&r, x);
-    return static_cast<int>(r);
+    // Win32 and Win64 expectations.
+    static_assert(sizeof(unsigned long)==4,"");
+    static_assert(sizeof(DWORD)==4,"");
+    DWORD where;
+    // Search from LSB to MSB for first set bit.
+    // Returns zero if no set bit is found.
+    if (_BitScanReverse(&where, mask))
+        return static_cast<int>(31-where);
+    return 32; // Undefined Behavior.
 }
-#endif // !__clang__
-#endif // _LIBCPP_MSVCRT
 
+_LIBCPP_ALWAYS_INLINE int __builtin_clz( unsigned int x )
+{
+    // Win32 and Win64 expectations.
+    static_assert(sizeof(unsigned int)==4,"");
+    return __builtin_clzl(x);
+}
+#endif // _LIBCPP_MSVC
+
 #endif // _LIBCPP_SUPPORT_WIN32_SUPPORT_H
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bittests.cpp
Type: text/x-c++src
Size: 2613 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131224/baf8ccf3/attachment.cpp>


More information about the cfe-commits mailing list