Page MenuHomePhabricator

[lld] Infer the driver mode from the target.
Needs RevisionPublic

Authored by danalbert on Apr 16 2020, 2:07 PM.

Details

Reviewers
srhines
MaskRay
Summary

This is a largely untested work in progress as an alternative to
https://reviews.llvm.org/D76452. If we decide to go this route I'll
add the missing tests :)

Diff Detail

Event Timeline

danalbert created this revision.Apr 16 2020, 2:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2020, 2:07 PM
MaskRay added inline comments.Apr 16 2020, 2:30 PM
lld/tools/lld/lld.cpp
134

This checks -m for lld-link, wasm-ld and ld64 which not be great.

We can place the check inside:

#if __APPLE__
  // Use Darwin driver for "ld" on Darwin.
  if (progname == "ld")
    return Darwin;
#endif
MaskRay added subscribers: int3, ruiu.EditedApr 16 2020, 2:32 PM

@ruiu @int3 If we can't really get rid of the #if __APPLE__ block, I would hope we can use

#if __APPLE__
  // Use Darwin driver for "ld" on Darwin.
  if (progname == "ld") {
    if (targetMatches(v, [](StringRef s) { ... }) return Gnu;
    return Darwin;
  }
#endif

This should address the Android folks' problem that ld on Darwin acts as a Mach-O linker (instead of an ELF linker). The approach seems cleaner than D76452.

-m is an ld64 obsoleted option.

-m          Don't treat multiple definitions as an error.  This is no
               longer supported. This option is obsolete.
danalbert marked an inline comment as done.Apr 16 2020, 3:01 PM
danalbert added inline comments.
lld/tools/lld/lld.cpp
134

Okay, if we don't want this behavior to affect non-Android targets I think the Clang patch is the right answer.

Although removing the Darwin case also SGTM.

MaskRay requested changes to this revision.Jul 28 2020, 12:00 AM

We went with D78837. This can be closed.

This revision now requires changes to proceed.Jul 28 2020, 12:00 AM