[clang] [Clang][Cygwin] Remove erroneous _WIN32 define and clean up Cygwin code (PR #138120)

[Martin Storsjö via cfe-commits]

mstorsjo wrote:

> If I grep through `/usr` for `_aligned_malloc` there are no matches for GCC and Clang guards it based on `_WIN32`:
> 
> ```
> $ grep -R _aligned_malloc /usr -C2
> /usr/lib/clang/20/include/mm_malloc.h-  void *__mallocedMemory;
> /usr/lib/clang/20/include/mm_malloc.h-#if defined(__MINGW32__)
> /usr/lib/clang/20/include/mm_malloc.h:  __mallocedMemory = __mingw_aligned_malloc(__size, __align);
> /usr/lib/clang/20/include/mm_malloc.h-#elif defined(_WIN32)
> /usr/lib/clang/20/include/mm_malloc.h:  __mallocedMemory = _aligned_malloc(__size, __align);
> /usr/lib/clang/20/include/mm_malloc.h-#else
> /usr/lib/clang/20/include/mm_malloc.h-  if (posix_memalign(&__mallocedMemory, __align, __size))
> --
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/x86_64-pc-msys/bits/c++config.h-/* #undef _GLIBCXX_HAVE__ACOSL */
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/x86_64-pc-msys/bits/c++config.h-
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/x86_64-pc-msys/bits/c++config.h:/* Define to 1 if you have the `_aligned_malloc' function. */
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/x86_64-pc-msys/bits/c++config.h-/* #undef _GLIBCXX_HAVE__ALIGNED_MALLOC */
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/x86_64-pc-msys/bits/c++config.h-
> --
> /usr/share/aclocal/ax_check_page_aligned_malloc.m4-# =================================================================================
> /usr/share/aclocal/ax_check_page_aligned_malloc.m4:#  https://www.gnu.org/software/autoconf-archive/ax_check_page_aligned_malloc.html
> /usr/share/aclocal/ax_check_page_aligned_malloc.m4-# =================================================================================
> /usr/share/aclocal/ax_check_page_aligned_malloc.m4-#
> ```
> 
> GCC has some `_WIN32` checks with special `__CYGWIN__` condition, but I think these code paths (except `fs_path.h`) are not that relevant:
> 
> ```
> $ grep -R _WIN32 /usr/lib/gcc
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/bits/fs_path.h:#if defined(_WIN32) && !defined(__CYGWIN__)
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/experimental/bits/fs_path.h:#if defined(_WIN32) && !defined(__CYGWIN__)
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/experimental/internet:#if defined _WIN32 && __has_include(<ws2tcpip.h>)
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/parallel/compatibility.h:#if !defined(_WIN32) || defined (__CYGWIN__)
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/parallel/compatibility.h:#if defined (_WIN32) && !defined (__CYGWIN__)
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/x86_64-pc-msys/bits/c++config.h:/* #undef _GLIBCXX_USE_WIN32_SLEEP */
> /usr/lib/gcc/x86_64-pc-msys/13.3.0/include/c++/x86_64-pc-msys/bits/os_defines.h:#define _GLIBCXX_THREAD_ATEXIT_WIN32 1
> ```
> 
> It would be possible to apply similar `&& !defined(__CYGWIN__)` here, but I don't know how many other places would hit the same problem.

I agree; it may be a way to go overall, but not necessarily the primary solution here (and it may end up requiring many more tweaks).

> While using Win32 APIs is sometimes necessary, I think defining `_WIN32` is a red flag, although I don't have that much experience with Cygwin.

Yes, I think so too. In this case here, the history seems to be a bit more tricky, as this originated as `#define LLVM_ON_WIN32 1` which may have been a bit more kosher to do, than an outright `#define _WIN32 1`. The original case here might have been fine, when 1865df49960e34cc90d0083b0e0cd4771c0feb35 got rid of `LLVM_ON_WIN32` we ended up with a more problematic case here.

> > I presume that the removed codepaths did serve some purpose at some point initially. Can we dig up that purpose from the git history and recheck whether it's needed/relevant still at this point.
> 
> It was introduced by [aa63fdf](https://github.com/llvm/llvm-project/commit/aa63fdf3b57ab5701e951b4bf20a9dca3d8da419)
> 
> I'd expect Cygwin API missing the required bits back then, these days it can use the same code as Linux in this file. @jeremyd2019 pointed that out in: [#134494 (comment)](https://github.com/llvm/llvm-project/pull/134494#discussion_r2035957416)

Ah, I hadn't read the earlier discussions of these patches (or I have read and promptly forgot), now I understand this a bit better. So yeah, I would also first have taken the direction of just removing the `#define _WIN32`.

Anyway, the comments from @jeremyd2019 there seal the case here I think. So if you update the PR description (i.e. future commit message) explaining why this no longer is necessary, I think this PR is good to go!

https://github.com/llvm/llvm-project/pull/138120