mati865 wrote: > Defining `_WIN32` in cygwin mode compilations, and including w32api headers > from there, feels brittle; I'm not familiar enough with cygwin to know if > this is something that one usually can do, or whether it is commonly done, or > what caveats that involves (doesn't e.g. cygwin have an incompatible > definition of the type `long`?) - so getting rid of that probably is good in > itself.
The rule of thumb is to avoid using Win32 APIs unless absolutely necessary, but I'm not familiar with the reasons. > Was this an issue only when building Clang with Clang - did this succeed if > building Clang with GCC? Indeed, it builds fine with GCC. I think the difference here is Clang being a cross-compiler, has more robust headers that cover all targets. GCC can drop bits irrelevant to its target at the build time. 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. 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. > 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 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: https://github.com/llvm/llvm-project/pull/134494#discussion_r2035957416 > While building may be broken right now, the individual issue of missing > `_aligned_malloc()` probably is fixable as well, so I'd like to know a bit > more about the reasoning for why this cygwin specific codepath should be > removed. There is another error for `_aligned_free` but I didn't include it to shorten the message. https://github.com/llvm/llvm-project/pull/138120 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
