This is an archive of the discontinued LLVM Phabricator instance.

gn build: Add a stage2 toolchain for Android.
ClosedPublic

Authored by pcc on Jan 10 2019, 9:37 PM.

Details

Summary

This makes it possible to build llvm-symbolizer for Android, which
is one of the prerequisites for running the sanitizer tests on Android.

Depends on D56427

Depends on D56575

Depends on D56576

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Jan 10 2019, 9:37 PM
thakis accepted this revision.Jan 11 2019, 6:40 AM

Super cool!

llvm/utils/gn/build/libs/pthread/BUILD.gn
9 ↗(On Diff #181208)

Maybe add a # On Android, bionic has built-in support for pthreads. like comment (I had to look that up just now).

llvm/utils/gn/build/toolchain/BUILD.gn
49 ↗(On Diff #181208)

(this might be a git merge issue -- be sure to run git ls-files '*.gn' '*.gni' | xargs -n 1 gn format though :-)

158 ↗(On Diff #181208)

Whoa, do deps on toolchains work now? Is that new?

169 ↗(On Diff #181208)

That's alright for now, but I think eventually we want to remove the target_foo stuff from the toolchain definition again and instead use configs for this (like https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?type=cs&q=build/config/compiler&sq=package:chromium&g=0&l=217)

172 ↗(On Diff #181208)

The -fuse-ld=lld will probably need revisiting -- probably want to also add that on Linux when e.g. clang_base_path is set, and maybe if some use_lld arg is explicitly set to on. But ok for now.

This revision is now accepted and ready to land.Jan 11 2019, 6:40 AM
phosek added inline comments.Jan 11 2019, 7:45 AM
llvm/utils/gn/build/toolchain/BUILD.gn
158 ↗(On Diff #181208)

It's been around for a while, but it's discouraged. gn help toolchain says for toolchain deps:

This concept is somewhat inefficient to express in Ninja (it requires a lot
of duplicate of rules) so should only be used when absolutely necessary.

I don't think there's a cleaner way for multi-stage builds though since you need to somehow express that dependency.

169 ↗(On Diff #181208)

That would be my preference. Rather than combining different ways of specifying flags, we should just introduce default configs for each platform and then add those to defaults for each target type (e.g. https://fuchsia.googlesource.com/build/+/master/config/BUILDCONFIG.gn#146)

phosek added inline comments.Jan 11 2019, 7:48 AM
llvm/utils/gn/build/toolchain/BUILD.gn
172 ↗(On Diff #181208)

I think that should also be a config that's set as a default. If it's done that way, targets can then opt-out by using:

configs -= ["//build/config:lld"]
pcc marked 3 inline comments as done.Jan 11 2019, 3:22 PM
pcc added inline comments.
llvm/utils/gn/build/toolchain/BUILD.gn
49 ↗(On Diff #181208)

Yes, I think I might have forgotten to format D56576. This change restores the correct formatting though.

169 ↗(On Diff #181208)

We'll see how that goes. One issue is that we'll need some way of writing the target flags to a file to run the tests:
http://llvm-cs.pcc.me.uk/projects/compiler-rt/test/hwasan/lit.site.cfg.in#5
and I'm not sure how easy that will be with a config.

This revision was automatically updated to reflect the committed changes.