Page MenuHomePhabricator

[gn] Support for building runtimes
AbandonedPublic

Authored by phosek on Apr 3 2019, 11:39 PM.

Details

Reviewers
thakis
Summary

This is (an incomplete) support for building compiler-rt builtins,
libunwind, libc++abi and libc++. The library build should be complete,
but not all CMake options have been replicated in GN.

We always use the just built compiler to build all the runtimes, which
is equivalent to the CMake runtimes build. This simplifies the build
configuration because we don't need to support arbitrary host compiler
and can always assume the latest Clang. With GN's toolchain support,
this is significantly more efficient than the CMake runtimes build.

Diff Detail

Event Timeline

phosek created this revision.Apr 3 2019, 11:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2019, 11:39 PM

This is still incomplete, most notably the tests are completely missing, and not every option is supported. I also haven't yet done any testing on macOS or Windows. Despite that, it should be sufficient to produce a working runtimes for Linux and so it might be reasonable to land this and then iterate given the size of this patch.

pcc added a subscriber: pcc.Apr 3 2019, 11:50 PM
pcc added inline comments.Apr 4 2019, 9:04 AM
llvm/utils/gn/secondary/clang/runtimes.gni
4

In http://llvm-cs.pcc.me.uk/utils/gn/secondary/compiler-rt/target.gni we have crt_current_out_dir which is basically the same as this.

IIUC this is the "new" style of runtime paths which is currently only used by Fuchsia. Should the code there be changed to do "if (is_fuchsia) use the new-style paths else use the old-style paths"?

llvm/utils/gn/secondary/libcxx/src/BUILD.gn
4

Do you really need all of these arguments? I would recommend only implementing the default behavior for most of them except for the ones that you actually need.

(Same comment elsewhere.)

159

I think this code is basically dead because the rest of the build system doesn't support Solaris.

Cool! Do you think we could maybe do one patch for unwind, libcxxabi, libcxx each? (Probably in that order.)

phosek marked 3 inline comments as done.Apr 5 2019, 11:25 AM
phosek added inline comments.
llvm/utils/gn/secondary/clang/runtimes.gni
4

The difference is that crt_current_out_dir applies only to compiler-rt while runtimes_dir applies to libunwind/libc++abi/libc++ and soon omp as well.

I don't think it's Fuchsia specific, it's true that we're the main user, but as discussed in the RFC: Place libs in Clang-dedicated directories (affects openmp, libcxx, libunwind, compiler-rt), there seems to be an agreement about making that the default, this layout is also used by other clients, e.g. Google internally. I'm fine putting it behind an argument, e.g. clang_enable_per_target_runtime_dir the same as we do in the CMake build.

llvm/utils/gn/secondary/libcxx/src/BUILD.gn
4

We need these for Fuchsia and/or libFuzzer (which is also needed for Fuchsia).

pcc added inline comments.Apr 5 2019, 12:10 PM
llvm/utils/gn/secondary/libcxx/src/BUILD.gn
4

Could you add them when you add Fuchsia and/or libFuzzer support, then? I feel like there ought to be a better way of making these customizations. For example, you could make the Fuchsia customizations conditional on current_os == "fuchsia", and the libc++ that libFuzzer uses could be built inside a custom toolchain with something like is_libfuzzer_build set and all of the libFuzzer customizations would be conditional on that.

phosek abandoned this revision.May 2 2019, 10:27 AM

Cool! Do you think we could maybe do one patch for unwind, libcxxabi, libcxx each? (Probably in that order.)

D60328
D60329
D60330
D60370
D60372
D61143