Page MenuHomePhabricator

[CMake] Filter libc++abi and libunwind from runtimes build in MSVC
ClosedPublic

Authored by phosek on Jan 31 2020, 2:58 PM.

Details

Summary

These don't build in MSVC at the moment, so filter these out altogether
from the list of runtimes and print a warning.

Diff Detail

Event Timeline

phosek created this revision.Jan 31 2020, 2:58 PM
Herald added a project: Restricted Project. · View Herald Transcript

I'm not sure if there's a better way. I cannot remove these from LLVM_ENABLE_RUNTIMES since I still want to build these for other targets, but I don't want these for Windows because they break the build.

Just for clarity, can you clarify in the subject that this is for windows/msvc (as I see in the code that you keep allowing building them for mingw - thanks for that!).

(I still only build these libs as standalone builds myself, but maybe I'll have time to try integrated building at some point.)

phosek retitled this revision from [CMake] Filter libc++abi and libunwind from runtimes build on Windows to [CMake] Filter libc++abi and libunwind from runtimes build on Windows/MSVC.Feb 3 2020, 11:42 AM
phosek edited the summary of this revision. (Show Details)

We added support for overriding LLVM_ENABLE_RUNTIMES on a per-target basis for this exact reason in D67195. Does that work for you?

(I want to generalize that support a bit, but it should already work for LLVM_ENABLE_RUNTIMES.)

phosek added a comment.Feb 3 2020, 2:28 PM

We added support for overriding LLVM_ENABLE_RUNTIMES on a per-target basis for this exact reason in D67195. Does that work for you?

(I want to generalize that support a bit, but it should already work for LLVM_ENABLE_RUNTIMES.)

Not out-of-the-box because that filter is only used for explicit targets, not for the "default" builtins and runtimes (i.e. host) which is what we currently use on Windows and Darwin.

smeenai accepted this revision.Feb 3 2020, 2:31 PM

In that case, looks good with the condition changed to MSVC.

llvm/runtimes/CMakeLists.txt
30–31

I think MSVC is a better condition here than WIN32 AND NOT MINGW, since libunwind and libc++abi aren't compatible with the MSVC ABI.

This revision is now accepted and ready to land.Feb 3 2020, 2:31 PM
phosek updated this revision to Diff 242674.Feb 5 2020, 9:58 AM
phosek marked 2 inline comments as done.
phosek retitled this revision from [CMake] Filter libc++abi and libunwind from runtimes build on Windows/MSVC to [CMake] Filter libc++abi and libunwind from runtimes build in MSVC.
phosek edited the summary of this revision. (Show Details)
phosek added inline comments.
llvm/runtimes/CMakeLists.txt
30–31

Done, I've also added a warning so it's easier to spot in the output.

This revision was automatically updated to reflect the committed changes.

Hello @phosek,

these changes break the ARM cross builders on Windows:
http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/4240

also it suppresses building of the libraries on the Aarch64 cross builder http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64

We use "just built" clang toolchain to build the runtime/builtin libraries there. These changes do not allow using the toolchain in case it was built by MSVC.

Would you fix it or revert the commit?

Hello @phosek,

the builders are broken for the third day in a row.
Sorry, but I'm going to revert this commit.

Hello @phosek,

the builders are broken for the third day in a row.
Sorry, but I'm going to revert this commit.

No worries and sorry about the late response, I missed your earlier message. I'll take a look into this issue.