[PATCH] D97451: [PR48898][CMake] Support MinGW Toolchain tools in llvm_ExternalProject_Add

Thu Feb 25 02:14:09 PST 2021

zero9178 marked an inline comment as done.
zero9178 added a comment.

In D97451#2587089 <https://reviews.llvm.org/D97451#2587089>, @mstorsjo wrote:

>> Windows is in the unique position of having two drivers, clang-cl and normal GNU clang, depending on whether a GNU or MSVC target is used.
>
> FWIW, the normal GNU frontend also works for the MSVC target; some finer details are omitted in that case, but it rougly does work (and if the default target of the build is an msvc target, that's what targeted when calling that frontend too).
>
> Also FWIW, I haven't really used the runtimes build mechanism at all, because I primarily bootstrap everything from scratch in a cross compile setup, i.e. with no preexisting build tools; first building the compiler, then building the base mingw crt, the compiler-rt builtins on top of that, and then other runtimes like libunwind, libcxxabi and libcxx on top of that. In such cases, afaik the runtimes build is a bit tricky, because I'd need to build other bits (e.g. the mingw crt) between building the compiler and the rest of the llvm provided runtimes. For now I just build all of those subprojects individually afterwards.
>
> So therefore all of this is a bit of uncharted territory for me.
>
> There's also a separate contender for how to build runtimes (still not entirely formally supported), as a middle ground - see https://github.com/llvm/llvm-project/blob/main/libcxx/utils/ci/runtimes/CMakeLists.txt. With that one, you configure building the runtimes just like a normal standalone project (i.e. you set the cmake configuration yourself, instead of instructing the machinery to figure it out based on a target triple), but you can add a series of projects to build as e.g. `-DLLVM_ENABLE_PROJECTS="libunwind;libcxxabi;libcxx"`. That setup has worked fine for me in some initial testing.

I use a MinGW sysroot that is already prebuilt (with GCC in my case) therefore only needing all the other runtimes to be built by clang. Additionally I also build sanitizers and others with clang-cl for i386,x86 and aarch64. I do all of this natively from Windows and being able to simply add LLVM_ENABLE_RUNTIMES to the normal cmake config makes it a lot easier and I only use a single cmake cache file containing all variables, nothing else.

I took heavy inspiration from some of Fuchsia build caches as seen here https://github.com/llvm/llvm-project/blob/d748908fa02b11c7840a7f03c7a52223126bdba9/clang/cmake/caches/Fuchsia.cmake. 
I then created my own script (https://gist.github.com/zero9178/cb992f102597f591dc7ed91449f77743) allowing me to just do `cmake .. -GNinja -C build.cmake` and it'll build a vast array of configurations.

The fact that it worked in LLVM 11 and before was more luck than anything. Since LLVM 12 this has been broken for mingw triples however, so I am trying to fix it.

================
Comment at: llvm/cmake/modules/LLVMExternalProjectUtils.cmake:56
       string(REGEX REPLACE "^-DCMAKE_SYSTEM_NAME=(.*)$" "\\1" _cmake_system_name "${arg}")
+    elseif (arg MATCHES "^-DCMAKE_(C|ASM|CXX)_COMPILER_TARGET=(.*)-windows-(gnu|msvc).*$")
+      if (${CMAKE_MATCH_3} STREQUAL "msvc")
----------------
mstorsjo wrote:
> Technically, you can have triples in other forms too, like `x86_64-win32`; `win32` is an alias for `windows`, and if the `-msvc` suffix is omitted it defaults to the msvc variant.
> 
> If this only deals with canonicalized triples it should be fine. And for other cases, I guess it might be fine as well (as you'd get proper results if you pass triples in the expected form at least and we can't expect to mimic all the quirks handled by the llvm internal triple parser).
I was checking around if there were any functions in other cmake modules or so that could parse or canonicalize triples. I was thinking that for now only supporting canonicalized triples should be fine. Other patterns could technically be added in the future. As far as I am aware, clang would also accept eg. GCC style triple for mingw as well I believe. 

================
Comment at: llvm/cmake/modules/LLVMExternalProjectUtils.cmake:74
     # AIX 64-bit XCOFF and big AR format is not yet supported in some of these tools.
-    if(NOT _cmake_system_name STREQUAL AIX)
+    if(NOT _cmake_system_name STREQUAL AIX OR use_mingw_toolchain_tools)
       list(APPEND ARG_TOOLCHAIN_TOOLS lld llvm-ar llvm-ranlib llvm-nm llvm-objdump)
----------------
mstorsjo wrote:
> I'm not quite sure I follow the logic of this condition (parentheses might help?) - is it `not (aix or mingw)` or `(not aix) or mingw`? And why doesn't the current default logic work for the mingw case? At worst it might build an unnecessary llvm-lib frontend?
That's true, that was a mistake of mine. Not a lot that can go wrong here but I'll only conditionally add llvm-lib anyway for the sake of it. Will fix shortly

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97451/new/

https://reviews.llvm.org/D97451