- Using LLVM_WINSYSROOT would pick up DIA SDK path automatically, otherwise llvm-pdbutil has no DIA support.
- Add MSVC_VER to specify VC tools version.
- Make MSVC_VER/WINSDK_VER optional. If not specified, use the highest version number like the driver does.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
/winsysroot is somewhat new, but I suppose that's fine (?)
Looks fine to me (haven't tried it though). The only real question below is where and how winsysroot gets added as flag :)
llvm/cmake/platforms/WinMsvc.cmake | ||
---|---|---|
212 | clang does this internally if they're not defined. Maybe add -vctoolsversion only if MSVC_VER is defined, and -winsdkversion only if WINSDK_VER is defined? | |
217 | MSVC_VER and WINSDK_VER are optional, so you probably should only check for the sysroot | |
221 | given that this is only set to check for existence, maybe this variable (and the check against it) isn't needed? | |
223 | same here | |
266 | Do you want to add /winsysroot down here too? Or how is that flag getting added? | |
288 | FWIW in Chromium we build our lld-link against libxml2 (with almost everything disabled) these days, and it can do /manifest handling without mt.exe if build that way. |
Thanks for the review!
llvm/cmake/platforms/WinMsvc.cmake | ||
---|---|---|
212 | yeah, considered that. Later I found out I still need MSVC_VER and WINSDK_VER to specify libpath for the linker. Guess we need to wait until lld-link accepts /winsysroot... | |
217 | same. Optional for the user, still need it to specify linker libpath. | |
266 | ||
288 | Thanks for the info. I'll try removing it locally. |
llvm/cmake/platforms/WinMsvc.cmake | ||
---|---|---|
288 | Oh, okay. It seems LLVM Windows release build does not link libxml2 yet. Is it possible to upstream your code (https://chromium.googlesource.com/chromium/src/+/66ff126e39334b72ec6f23b6d28a60e7efbbc019/tools/clang/scripts/build.py#281) to llvm/utils/release/build_llvm_package.bat so the release lld-link works with /manifest? |
llvm/cmake/platforms/WinMsvc.cmake | ||
---|---|---|
266 | Not all parts of LLVM's build call HandleLLVMOptions, e.g. compiler-rt doesn't. We should add the flag explicitly to handle those cases as well (and I don't think having it specified twice should cause any harm). I'll put up a patch. |
In case of Visual Studio 2019/2022, Windows SDK are installed outside of MSVS directory
C:\Program Files (x86)\Windows Kits\10
I believe that was the case for earlier versions as well (at least 2015 and 2017), but this is intended for cross-compilation, so the idea was that you'd arrange the SDK and VC directories in the layout expected by /winsysroot.
clang does this internally if they're not defined. Maybe add -vctoolsversion only if MSVC_VER is defined, and -winsdkversion only if WINSDK_VER is defined?