[PATCH] D141738: [WIP] Add initial support for cross compile Windows runtimes under Linux when building Fuchsia clang toolchain

[Petr Hosek via Phabricator via cfe-commits]

phosek added inline comments.


================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:73
+
+if(WIN32 OR (LINUX AND WINDOWS_SDK_DIR))
+#if(WINDOWS_SDK_DIR)
----------------
I'd drop this part altogether, let's build these anytime when `WINDOWS_SDK_DIR` is set.


================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:96-99
+        -Xclang
+        -ivfsoverlay
+        -Xclang
+        "${WINDOWS_SDK_DIR}/llvm-vfsoverlay.yaml"
----------------
I think we should introduce a new LLVM CMake option for setting this akin to `LLVM_WINSYSROOT` (for example `LLVM_VFSOVERLAY`).


================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:100-101
+        "${WINDOWS_SDK_DIR}/llvm-vfsoverlay.yaml"
+        /winsysroot
+        ${WINDOWS_SDK_DIR})
+    string(REPLACE ";" " " CLANG_WINDOWS_CROSS_FLAGS "${CLANG_WINDOWS_CROSS_FLAGS}")
----------------
Can we instead set `BUILTINS_${target}_LLVM_WINSYSROOT` to `${WINDOWS_SDK_DIR}`?


================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:105-108
+        -libpath:"${WINDOWS_SDK_DIR}/VC/Tools/MSVC/14.34.31933/lib/x64"
+        -libpath:"${WINDOWS_SDK_DIR}/VC/Tools/MSVC/14.34.31933/atlmfc/lib/x64"
+        -libpath:"${WINDOWS_SDK_DIR}/Windows Kits/10/Lib/10.0.19041.0/ucrt/x64"
+        -libpath:"${WINDOWS_SDK_DIR}/Windows Kits/10/Lib/10.0.19041.0/um/x64")
----------------
These should be set automatically if you set `/winsysroot` (through `LLVM_WINSYSROOT`), see https://github.com/llvm/llvm-project/blob/a64846bee0bb4b4912c8cf6bf018ba5d892065d1/clang/lib/Driver/ToolChains/MSVC.cpp#L107.


================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:144-145
+        "${WINDOWS_SDK_DIR}/llvm-vfsoverlay.yaml"
+        /winsysroot
+        ${WINDOWS_SDK_DIR})
+    string(REPLACE ";" " " CLANG_WINDOWS_CROSS_FLAGS "${CLANG_WINDOWS_CROSS_FLAGS}")
----------------
Can we instead set `RUNTIMES_${target}_LLVM_WINSYSROOT` to `${WINDOWS_SDK_DIR}`?


================
Comment at: llvm/cmake/modules/LLVMExternalProjectUtils.cmake:172
+                          -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang-cl${CMAKE_EXECUTABLE_SUFFIX}
+                          -DCMAKE_RC_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-rc
+                          -DCMAKE_MT=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-mt
----------------
This should be in a separate block just like other tools below guarded by a `llvm-rc IN_LIST TOOLCHAIN_TOOLS` condition.


================
Comment at: llvm/cmake/modules/LLVMExternalProjectUtils.cmake:173
+                          -DCMAKE_RC_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-rc
+                          -DCMAKE_MT=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-mt
+                          )
----------------
This should be in a separate block just like other tools below guarded by a `llvm-mt IN_LIST TOOLCHAIN_TOOLS` condition. 


================
Comment at: llvm/runtimes/CMakeLists.txt:359
 
-  if(NOT RUNTIMES_${name}_LLVM_USE_LINKER AND NOT RUNTIMES_${target}_LLVM_USE_LINKER)
+  if(NOT RUNTIMES_${name}_LLVM_USE_LINKER AND NOT RUNTIMES_${target}_LLVM_USE_LINKER AND NOT ${target} STREQUAL "x86_64-pc-windows-msvc")
     list(APPEND ${name}_extra_args -DLLVM_USE_LINKER=${LLVM_USE_LINKER})
----------------
We should fix the build so that this isn't needed, it should be possible to use `LLVM_USE_LINKER=lld` when targeting Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141738