This is an archive of the discontinued LLVM Phabricator instance.

[runtimes][cmake] Propagate cmake arguments.
Needs ReviewPublic

Authored by Mordante on Aug 25 2023, 12:34 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

The documentation [1] has an example

$ cmake -G Ninja -S runtimes -B build \

-DLLVM_ENABLE_RUNTIMES="libcxx"           \
-DLIBCXX_CXX_ABI=libstdc++                \
-DLIBCXX_CXX_ABI_INCLUDE_PATHS="/usr/include/c++/4.7/;/usr/include/c++/4.7/x86_64-linux-gnu/"

Unfortunately the LIBCXX_ cmake arguments aren't forwarded to the CMake
for libc++. This was discovered while working on enabling modules in the
libc++ bootstrap CI build.

[1] https://libcxx.llvm.org/BuildingLibcxx.html#using-libsupc-on-linux

Depends on D158860

Diff Detail

Event Timeline

Mordante created this revision.Aug 25 2023, 12:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 25 2023, 12:34 PM
Mordante requested review of this revision.Aug 25 2023, 12:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 25 2023, 12:34 PM

@phosek Didn't we already support passing e.g. LIBCXX_ENABLE_FOO variables through the bootstrapping build? I can't find where that's done though.

phosek added a comment.Sep 6 2023, 9:59 AM

@phosek Didn't we already support passing e.g. LIBCXX_ENABLE_FOO variables through the bootstrapping build? I can't find where that's done though.

We do for the default target, see my comment. @Mordante Can you share the full CMake invocation?

llvm/runtimes/CMakeLists.txt
353

This code path is only used with LLVM_RUNTIME_TARGETS, passing through all variables into every target is a breaking change because not all variables might be applicable to every target.

398

We already passthrough prefixes into the default target here.

Mordante added inline comments.Sep 6 2023, 10:52 AM
llvm/runtimes/CMakeLists.txt
398

I tried this the the option -DLIBCXX_ENABLE_STD_MODULES=ON in D158934. Should that patch work "out-of-the-box"? If so I can retest that patch without this one.

phosek added inline comments.Sep 6 2023, 11:19 AM
llvm/runtimes/CMakeLists.txt
398

It should, let me know if it doesn't work and I can take a look.