[builtins] Enabled emulated TLS on WOA.

[martell]

Summary

Thanks to recent work from @marsupial here rL301350
We can now bootstrap mingw x86 and x64 with clang without importing winpthreads headers. .i.e pthread.h

I never enabled emutls on WOA because of this extra step previously.
@mstorsjo would enabling this cause any issues with your work?

I also changed Windows.h to windows.h to support mingw from file case sensitive systems.
MSVC doesn't care much about this because it is run from windows.

I added Saleem also because he cares about cross compiling to windows and can give another perspective on the case sensitivity.

[martell]

Added diff context for readability

[compnerd]

Doing this on just the MinGW build seems reasonable. Most of the library shouldn't be needed for Windows.

lib/builtins/emutls.c

101

The file is Windows.h in the SDK, so why are you changing it to the lower case?

[martell]

In D32681#741979, @compnerd wrote:

The file is Windows.h in the SDK, so why are you changing it to the lower case?

Mingw headers do not follow the uppercase naming conventions of the windows sdk.
Projects that are typically build from not windows platforms follow the mingw convention when supporting both msvc and mingw.
Example https://github.com/videolan/vlc/blob/master/modules/video_output/win32/common.c#L38

The reason for me changing to lowercase is that MSVC is case insensitive on windows so it should not affect the target, while fixing cross building mingw at the same time.
The only downside to using lowercase W is someone would be restricted if they tried to use the MSVC SDK from a non windows machine that is case sensitive.

[martell]

@chapuni tends to deal with all mingw on linux stuff from reading here https://reviews.llvm.org/D30107

[mstorsjo]

In D32681#742185, @martell wrote:

In D32681#741979, @compnerd wrote:

The file is Windows.h in the SDK, so why are you changing it to the lower case?

Mingw headers do not follow the uppercase naming conventions of the windows sdk.
Projects that are typically build from not windows platforms follow the mingw convention when supporting both msvc and mingw.
Example https://github.com/videolan/vlc/blob/master/modules/video_output/win32/common.c#L38

The reason for me changing to lowercase is that MSVC is case insensitive on windows so it should not affect the target, while fixing cross building mingw at the same time.
The only downside to using lowercase W is someone would be restricted if they tried to use the MSVC SDK from a non windows machine that is case sensitive.

FWIW, the official Windows SDK isn't internally consistent with this either - e.g. files are included with a different variant of upper/lowercase than the actual filenames.

I'm using the windows SDK both with MSVC (run via wine) and with clang from linux on case sensitive filesystems, and to make this work properly, I normally end up lowercasing all the filenames (matching the mingw headers), despite starting out with intentions to modify it as little as possible.

[mstorsjo]

@mstorsjo would enabling this cause any issues with your work?

I haven't read up on all what this entails, but... When building for WoA in MSVC mode, I don't use compiler-rt/builtins at all, since it all should use the helper functions that MSVC uses. So this particular change shouldn't hurt me at all.

The projects I test on WoA doesn't use TLS at all either so I'm not sure if it is broken (or gets broken by any recent change) either though.

[compnerd]

@mstorsjo Yeah, clang supports proper TLS on Windows on ARM. This is only needed if you are explicitly going out of your way to use the emulated TLS (via -femulated-tls). Otherwise, this code path shouldn't be hit.

I have a local patch (well, it's either on reviews.llvm.org or on the mailing list as well) that emulates the case insensitivity, but that is expensive, and I'd rather keep the hit to the actual file when possible. How about adding a guard around the incorrect case (yes, Microsoft doesn't get the case right, but the SDK provides the file with the capital name, so that should be the official version IMO).

[chapuni]

In D32681#742912, @compnerd wrote:

I have a local patch (well, it's either on reviews.llvm.org or on the mailing list as well) that emulates the case insensitivity, but that is expensive, and I'd rather keep the hit to the actual file when possible. How about adding a guard around the incorrect case (yes, Microsoft doesn't get the case right, but the SDK provides the file with the capital name, so that should be the official version IMO).

I really wish you to stop following its capital file names. <windows.h> should be fine.

[martell]

In D32681#742912, @compnerd wrote:

I have a local patch (well, it's either on reviews.llvm.org or on the mailing list as well) that emulates the case insensitivity, but that is expensive, and I'd rather keep the hit to the actual file when possible. How about adding a guard around the incorrect case (yes, Microsoft doesn't get the case right, but the SDK provides the file with the capital name, so that should be the official version IMO).

Adding a Guard for having an upper and lower case windows.h include would be very ugly and is not done anywhere else in the codebase.
Here is a reference to another location where windows.h is included in LLVM.
https://github.com/llvm-mirror/llvm/blob/master/lib/Support/Windows/WindowsSupport.h#L47

Building for mingw i686 and x86_64 from linux is broken for me atm without this patch.
I just need the go ahead to push to fix that :)

[martell]

ping

[martell]

In D32681#742914, @chapuni wrote:

<windows.h> should be fine.

Oops, I missed @chapuni's reply.
Thanks

lib/builtins/CMakeLists.txt

       udivmoddi4.c
       udivmodsi4.c
       udivsi3.c
-      umoddi3.c)
+      umoddi3.c
+      emutls.c)
 elseif(NOT WIN32)
   # TODO the EABI sources should only be added to EABI targets
   set(arm_SOURCES

lib/builtins/emutls.c

 
 #else
 
-#include <Windows.h>
+#include <windows.h>
 #include <malloc.h>
 #include <stdio.h>
 #include <assert.h>