[patch] [libcxx] [win32] support.h
G M
gmisocpp at gmail.com
Mon Dec 23 12:22:26 PST 2013
Here's a revised version of my recent patch that implements yaron's
suggestion to remove DWORD use. I've also made it more explicitly
Win32/msvc specific which I think it practically was anyway and being
explicit may be easier to manage.
On Tue, Dec 24, 2013 at 8:45 AM, G M <gmisocpp at gmail.com> wrote:
> Hi Yaron
>
>
> On Tue, Dec 24, 2013 at 2:07 AM, Yaron Keren <yaron.keren at gmail.com>wrote:
>
>> Hi G M,
>>
>> The DWORDs should not have been there at all as _BitScanReverse64 and
>> _BitScanForward formally take unsigned long* and not DWORD* (although they
>> are the same thing).
>>
>> http://msdn.microsoft.com/en-us/library/wfd9z0bb(v=vs.90).aspx
>>
>> Use unsigned long instead of DWORD and wtypes.h will not be needed.
>>
>> Yes I find this confusing because I'm pretty sure other documentation
> (which I now conveniently can't find) had the types using DWORD etc. and I,
> and I'm sure the original author of these functions that I'm fixing, based
> their code after that and not the documentation you are linking too. That's
> how I think the DWORD stuff got in to begin with.
>
> In an earlier version of this patch that I posted, but seemed to get lost
> I removed the DWORD types etc. due to the documentation you linked to.
>
> What caused me to put it back was that some headers like Winnt.h still
> refer to DWORD etc. But now looking at intrin.h which I can see is the one
> to follow doesn't use DWORD anymore if it ever did.
>
> So I can see you are right, I'll re-submit another patch shortly that
> again removes DWORD.
>
> Thanks.
>
>>
>>
>>
>> 2013/12/23 G M <gmisocpp at gmail.com>
>>
>>> 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/493e03c2/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
+// "builtins" not implemented here for Clang or GCC as they provide implementations.
+// Assuming required for elsewhere else, certainly MSVC.
+#if defined(_LIBCPP_MSVC)
+#include <intrin.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,16 @@
{ return _Stold(nptr, endptr, 0); }
#define _Exit _exit
+#endif
-#ifndef __clang__ // MSVC-based Clang also defines _MSC_VER
-#include <intrin.h>
+#if defined(_LIBCPP_MSVC)
+// Bit builtin's make these assumptions when calling _BitScanForward/Reverse etc.
+// These assumptions are expected to be true for Win32/Win64 which this file supports.
+static_assert(sizeof(unsigned long long)==8,"");
+static_assert(sizeof(unsigned long)==4,"");
+static_assert(sizeof(unsigned int)==4,"");
+
_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 +93,87 @@
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);
+ unsigned long where;
+ // 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 )
+{
+ unsigned long 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);
+ unsigned long 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<unsigned long>(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<unsigned long>(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);
+ unsigned long 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 )
+{
+ return __builtin_clzl(x);
+}
+#endif // _LIBCPP_MSVC
+
#endif // _LIBCPP_SUPPORT_WIN32_SUPPORT_H
More information about the cfe-commits
mailing list