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

[G M]

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/638170fa/attachment.html>