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

[Yaron Keren]

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.

Yaron



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/20131223/88d432c4/attachment.html>