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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
