This is an archive of the discontinued LLVM Phabricator instance.

[gn build] Add check-lld target and make it work
ClosedPublic

Authored by thakis on Dec 18 2018, 9:51 AM.

Details

Summary

Also add a build file for llvm-lit, which in turn needs llvm/tools/llvm-config.

With this, check-lld runs and passes all of lld's lit tests. It doesn't run any of its unit tests yet.

Running just ninja -C out/gn will build all prerequisites needed to run tests, but it won't run the tests (so that the build becomes clean after one build). Running ninja -C out/gn check-lld will build prerequisites if needed and run the tests. The check-lld target never becomes clean and runs tests every time.

llvm-config's build file is a bit gnarly: Everything not needed to run tests is basically stubbed out. Also, to generate LibraryDependencies.inc we shell out to llvm-build at build-time. It would be much nicer to get the library dependencies by using the dependency data the GN build contains (http://llvm-cs.pcc.me.uk/gen/tools/llvm-config/LibraryDependencies.inc#1).

Diff Detail

Repository
rL LLVM

Event Timeline

thakis created this revision.Dec 18 2018, 9:51 AM

phosek: ping :-)

phosek added inline comments.Dec 19 2018, 10:46 AM
llvm/utils/gn/secondary/lld/test/BUILD.gn
8 ↗(On Diff #178715)

This is likely going to be useful elsewhere, do you plan to move into a .gni file later?

13 ↗(On Diff #178715)

Is this formatted with gn format? I'm surprised it doesn't put this onto a single line.

26 ↗(On Diff #178715)

Do we care about builds being relocatable? If yes then we should invoke rebase_path with root_build_dir as a second argument.

84 ↗(On Diff #178715)

Does lld really depend on these symlinks (the same below)? I don't see any uses of e.g. llvm-ranlib, llvm-strip in lld's test suite.

llvm/utils/gn/secondary/llvm/tools/llvm-config/BUILD.gn
49 ↗(On Diff #178715)

You can use not_needed for that purpose.

thakis updated this revision to Diff 178932.Dec 19 2018, 11:55 AM
thakis marked 5 inline comments as done.

not_needed

Thanks!

llvm/utils/gn/secondary/lld/test/BUILD.gn
8 ↗(On Diff #178715)

Right now I have one of these per {lld,llvm,clang}/test/BUILD.gn locally. The main thing in the template is the list of defines and that's somewhat different everywhere. (I'm also using a separate python script in my local branch, so I'm not 100% sure how upstream clang/test/BUILD.gn will look like. Let's revisit this when the clang test build file arrives.

13 ↗(On Diff #178715)

Yes, I run git ls-files '*.gn' '*.gni' | xargs -n 1 gn format every now and then (and just did again, no changes). I think gn puts one-element lists on one line after += but not after =.

26 ↗(On Diff #178715)

That's a great comment! lit currently wants absolute directories. I explored having relocatable build dirs in https://github.com/nico/llvm-project-20170507/tree/gn_swarmingdemo (the top commit has all relevant bits) and it's possible but needs some lit changes. So for now this needs to be as-is, sadly.

84 ↗(On Diff #178715)

I figured for targets having symlinks, it's simpler to always depend on the symlink target, since that's easy to remember. Don't feel strongly about it, though.

llvm/utils/gn/secondary/llvm/tools/llvm-config/BUILD.gn
49 ↗(On Diff #178715)

Oh, that's nicer. Done.

phosek accepted this revision.Dec 19 2018, 2:51 PM

LGTM

This revision is now accepted and ready to land.Dec 19 2018, 2:51 PM
This revision was automatically updated to reflect the committed changes.