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

[Haowei Wu via Phabricator via llvm-commits]

haowei marked 2 inline comments as done.
haowei added inline comments.


================
Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:73
+
+if(WIN32 OR (LINUX AND WINDOWS_SDK_DIR))
+#if(WINDOWS_SDK_DIR)
----------------
phosek wrote:
> I'd drop this part altogether, let's build these anytime when `WINDOWS_SDK_DIR` is set.
We need to do a soft transition if we drop the "WIN32" since the bots are not passing the WINSYSROOT flag yet. Let's keep the WIN32 here.


================
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")
----------------
phosek wrote:
> 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.
There is an issue that "CLANG_CL" variable is not correctly set when clang kicks runtime build. When CLANG_CL is not set, the /winsysroot will not be added by the LLVM_WINSYSROOT flag. I have some thoughts about it and prefer to address this issue in a follow up patch instead of put everything here.


================
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}")
----------------
phosek wrote:
> Can we instead set `RUNTIMES_${target}_LLVM_WINSYSROOT` to `${WINDOWS_SDK_DIR}`?
As stated in an earlier comment, this will require fixing the setting the "CLANG_CL" variable in compiler version detection. 


================
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})
----------------
phosek wrote:
> We should fix the build so that this isn't needed, it should be possible to use `LLVM_USE_LINKER=lld` when targeting Windows.
I made some other changes and now the /fuse-ld=lld will be passed to lld-link.exe if we leave this line unchanged. 
It will through a warning but won't stop the build.
Once I fix the CLANG_CL issue, I can prevent it from inserting `/fuse-ld=lld` when using CLANG_CL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141738