Page MenuHomePhabricator

Use LLD by default for Android.
AbandonedPublic

Authored by danalbert on Mar 19 2020, 1:39 PM.

Details

Reviewers
srhines
MaskRay

Diff Detail

Event Timeline

danalbert created this revision.Mar 19 2020, 1:39 PM
danalbert updated this revision to Diff 251498.Mar 19 2020, 4:24 PM
srhines accepted this revision.Mar 20 2020, 12:33 PM

Thanks, Dan, for setting this up.

This revision is now accepted and ready to land.Mar 20 2020, 12:33 PM
danalbert abandoned this revision.Mar 20 2020, 4:07 PM

Looks like we don't actually need this. Can achieve the same effect by installing ld.lld to the same directory as Clang as ld and it'll be preferred over the other locations.

Thanks for abandoning this patch. I am glad Android does not need customization on top of the generic Linux behavior. (This makes clangDriver clean.)

danalbert reclaimed this revision.Mar 26 2020, 12:23 PM

It seems I'd goofed something in my testing earlier (I think I still had -fuse-ld=lld force on in my build system). While Clang will find ld in the driver directory and prefer it, LLD defaults to the Darwin driver mode when argv[0] is ld when run on Darwin. We need GNU mode, and the best way to get this behavior is to have Clang invoke ld.lld instead.

This revision is now accepted and ready to land.Mar 26 2020, 12:23 PM

Another approach will be -DCLANG_DEFAULT_LINKER=lld. It provides a default value for -fuse-ld=.

Another approach will be -DCLANG_DEFAULT_LINKER=lld. It provides a default value for -fuse-ld=.

How does that work for Darwin builds? Is lld fully supported for MachO at this point, or will Clang still choose to use the preferred Apple linker?

MaskRay added a comment.EditedMar 26 2020, 2:43 PM

Another approach will be -DCLANG_DEFAULT_LINKER=lld. It provides a default value for -fuse-ld=.

How does that work for Darwin builds? Is lld fully supported for MachO at this point, or will Clang still choose to use the preferred Apple linker?

I am not clear about your use case. lld can be used as ELF/Mach-O/COFF/WebAssembly linker. If you want to build ELF objects, -fuse-ld=lld (via clangDriver) or configure clang with -DCLANG_DEFAULT_LINKER=lld.
If you want to build Mach-O objects, lld's current Mach-O port lacks features (and will be removed). The new Mach-O port is still under development.

To cross build ELF object on macOS, another alternative is a wrapper named ld which invokes lld -flavor gnu "$@"

Another approach will be -DCLANG_DEFAULT_LINKER=lld. It provides a default value for -fuse-ld=.

How does that work for Darwin builds? Is lld fully supported for MachO at this point, or will Clang still choose to use the preferred Apple linker?

I am not clear about your use case. lld can be used as ELF/Mach-O/COFF/WebAssembly linker. If you want to build ELF objects, -fuse-ld=lld (via clangDriver) or configure clang with -DCLANG_DEFAULT_LINKER=lld.
If you want to build Mach-O objects, lld's current Mach-O port lacks features (and will be removed). The new Mach-O port is still under development.

Right, so this is a problem when we ship a multi-target toolchain that does support targeting regular Linux, Windows, and Darwin (for host tools) in addition to Android. If we switched CLANG_DEFAULT_LINKER, then we would need to pass explicit flags to any Darwin builds to go back to using the system linker.

To cross build ELF object on macOS, another alternative is a wrapper named ld which invokes lld -flavor gnu "$@"

@danalbert Would this kind of idea work with your other reverted patch? I'm not sure exactly what broke on those builds.

grimar added a subscriber: grimar.Mar 27 2020, 2:28 AM

To cross build ELF object on macOS, another alternative is a wrapper named ld which invokes lld -flavor gnu "$@"

@danalbert Would this kind of idea work with your other reverted patch? I'm not sure exactly what broke on those builds.

Not really. Windows complicates things here, since it operates based on file extensions rather than shebangs. ld.exe is not ld.cmd, and ld (with no extension) doesn't work. Any system that expects one form won't work with the other. Beyond that, process creation is expensive on Windows so we should be avoiding as many superfluous processes as we can.

@MaskRay Any other ideas, or should I submit this? Reviewing all our options:

  1. Installing LLD as simply "ld"

Rejected: Causes LLD to act in mach-o mode for Darwin

  1. -DCLANG_DEFAULT_LINKER=lld

Rejected: Our host Darwin toolchain still uses the system's linker, not LLD, and this would change that too (and as you said, the mach-o support in LLD isn't ready and is about to be replaces, so we can't do this for our production toolchain).

  1. Using a wrapper script to set -flavor gnu

Rejected: Wrappers don't work well on Windows hosts.

  1. Teach LLD that Linux targets are -flavor gnu, regardless of host.

LLD doesn't seem to differentiate between Android and non-Android Linux, so this change would affect non-Android Linux targets as well. Is that a problem? Do non-Android Linux targets linked from Windows, Darwin, or WebASM want the host driver modes the target driver modes?

  1. This patch.
MaskRay accepted this revision.EditedApr 16 2020, 1:37 PM

LGTM.

But hope @ruiu or @int3 can clarify that we can't get rid of the __APPLE__ special case in:

// lld/tools/lld/lld.cpp
static Flavor parseProgname(StringRef progname) {
#if __APPLE__
  // Use Darwin driver for "ld" on Darwin.
  if (progname == "ld")
    return Darwin;
#endif

#if LLVM_ON_UNIX
  // Use GNU driver for "ld" on other Unix-like system.
  if (progname == "ld")
    return Gnu;
#endif
MaskRay added a subscriber: ruiu.Apr 16 2020, 1:38 PM
MaskRay added a subscriber: int3.

Option 4 was (at least on the surface) super easy: https://reviews.llvm.org/D78328. lmk if you'd prefer that approach. I'm slightly less confident in it since it affects non-Android platforms as well.

Can we use -DCLANG_DEFAULT_LINKER=lld to configure AOSP's distribution of LLD, then require the use of -fuse-ld=<whatever the host linker is on OSX that is currently used> when targeting OSX host tools?

Can we use -DCLANG_DEFAULT_LINKER=lld to configure AOSP's distribution of LLD, then require the use of -fuse-ld=<whatever the host linker is on OSX that is currently used> when targeting OSX host tools?

It'd work (and might be a good short term solution here until we can get feedback from @ruiu, and @int3) but I'm not sure I like the idea of breaking the default Darwin configuration much more than the default Android configuration. It's less commonly used by our toolchain, but it's definitely used.

int3 added a comment.Apr 24 2020, 1:40 PM

I don't think I have enough context here to answer the question, but I'm pretty sure that change wouldn't affect what I'm working on

I don't think I have enough context here to answer the question, but I'm pretty sure that change wouldn't affect what I'm working on

Sorry, wasn't referring to that question specifically, but the LLD one that @MaskRay CC'd you for a little further up.

But hope @ruiu or @int3 can clarify that we can't get rid of the APPLE special case in:

// lld/tools/lld/lld.cpp
static Flavor parseProgname(StringRef progname) {
#if __APPLE__
  // Use Darwin driver for "ld" on Darwin.
  if (progname == "ld")
    return Darwin;
#endif

#if LLVM_ON_UNIX
  // Use GNU driver for "ld" on other Unix-like system.
  if (progname == "ld")
    return Gnu;
#endif
int3 added a comment.Apr 24 2020, 3:11 PM

Yes, I was referring to that question too :) I'm working on the new lld-macho implementation, under the DarwinNew flavor. I'm not sure if anything depends on the old Darwin flavor, which is why we haven't removed it yet, though we plan to do that once we get the new implementation to a more mature stage.

Yes, I was referring to that question too :) I'm working on the new lld-macho implementation, under the DarwinNew flavor. I'm not sure if anything depends on the old Darwin flavor, which is why we haven't removed it yet, though we plan to do that once we get the new implementation to a more mature stage.

Ah, gotcha :) Thanks! Will wait for @ruiu to chime in.

Yes, I was referring to that question too :) I'm working on the new lld-macho implementation, under the DarwinNew flavor. I'm not sure if anything depends on the old Darwin flavor, which is why we haven't removed it yet, though we plan to do that once we get the new implementation to a more mature stage.

Ah, gotcha :) Thanks! Will wait for @ruiu to chime in.

I vote for deleting the #ifdef __APPLE__ chunk so we don't have to add more code to either clang or lld....
The code owner of the existing lld darwin has explicitly expressed that we can drop the existing Darwin flavor at any time.

danalbert abandoned this revision.Apr 24 2020, 4:18 PM

Yes, I was referring to that question too :) I'm working on the new lld-macho implementation, under the DarwinNew flavor. I'm not sure if anything depends on the old Darwin flavor, which is why we haven't removed it yet, though we plan to do that once we get the new implementation to a more mature stage.

Ah, gotcha :) Thanks! Will wait for @ruiu to chime in.

I vote for deleting the #ifdef __APPLE__ chunk so we don't have to add more code to either clang or lld....
The code owner of the existing lld darwin has explicitly expressed that we can drop the existing Darwin flavor at any time.

SGTM. Abandoning this. I'll send a patch to remove the LLD side shortly.

Yes, I was referring to that question too :) I'm working on the new lld-macho implementation, under the DarwinNew flavor. I'm not sure if anything depends on the old Darwin flavor, which is why we haven't removed it yet, though we plan to do that once we get the new implementation to a more mature stage.

Ah, gotcha :) Thanks! Will wait for @ruiu to chime in.

I vote for deleting the #ifdef __APPLE__ chunk so we don't have to add more code to either clang or lld....
The code owner of the existing lld darwin has explicitly expressed that we can drop the existing Darwin flavor at any time.

SGTM. Abandoning this. I'll send a patch to remove the LLD side shortly.

https://reviews.llvm.org/D78837