This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Switch default to new Darwin backend
ClosedPublic

Authored by int3 on Jan 21 2021, 10:42 PM.

Details

Reviewers
thakis
Group Reviewers
Restricted Project
Commits
rG415c0cd698a8: [lld-macho] Switch default to new Darwin backend
Summary

The new Darwin backend for LLD is now able to link reasonably large
real-world programs on x86_64. For instance, we have achieved
self-hosting for the X86_64 target, where all LLD tests pass when
building lld with itself on macOS. As such, we would like to make it the
default back-end.

The new port is now named ld64.lld, and the old port remains
accessible as ld64.lld.darwinold

This [annoucement email][1] has some context. (But note that, unlike
what the email says, we are no longer doing this as part of the LLVM 12
branch cut -- instead we will go into LLVM 13.)

Numerous mechanical test changes were required to make this change; in
the interest of creating something that's reviewable on Phabricator,
I've split out the boring changes into a separate diff (D95905). I plan to
merge its contents with those in this diff before landing.

(@gkm made the original draft of this diff, and he has agreed to let me
take over.)

[1]: https://lists.llvm.org/pipermail/llvm-dev/2021-January/147665.html

Diff Detail

Event Timeline

gkm created this revision.Jan 21 2021, 10:42 PM
gkm requested review of this revision.Jan 21 2021, 10:42 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 21 2021, 10:42 PM
gkm edited the summary of this revision. (Show Details)Jan 21 2021, 10:52 PM
gkm edited the summary of this revision. (Show Details)
gkm planned changes to this revision.Jan 22 2021, 5:19 PM

Pulling this back from review ... Not ready for prime time.

gkm edited the summary of this revision. (Show Details)Jan 22 2021, 5:21 PM
int3 commandeered this revision.Feb 2 2021, 3:50 PM
int3 added a reviewer: gkm.
int3 updated this revision to Diff 320940.Feb 2 2021, 3:52 PM
int3 retitled this revision from [lld-macho] switch default to new Darwin port to [lld-macho] Switch default to new Darwin port.
int3 edited the summary of this revision. (Show Details)
int3 edited reviewers, added: thakis; removed: gkm.

update

int3 updated this revision to Diff 321001.Feb 2 2021, 9:58 PM
int3 added a subscriber: gkm.

rebase

thakis accepted this revision.Feb 5 2021, 6:38 PM
This revision is now accepted and ready to land.Feb 5 2021, 6:38 PM
smeenai added a subscriber: smeenai.Feb 5 2021, 6:41 PM
smeenai added inline comments.
llvm/utils/gn/build/BUILD.gn
220–221

I'm pretty sure -fuse-ld=ld64.lld won't work ... the ld64. prefix is added by the clang driver itself. Just -fuse-ld=lld should do the trick (in other words, this conditional can go away entirely).

int3 updated this revision to Diff 321921.Feb 5 2021, 10:00 PM
int3 marked an inline comment as done.

fix

int3 retitled this revision from [lld-macho] Switch default to new Darwin port to [lld-macho] Switch default to new Darwin backend.Feb 6 2021, 1:09 PM
int3 edited the summary of this revision. (Show Details)
thakis added inline comments.Feb 8 2021, 12:31 PM
lld/tools/lld/lld.cpp
74

very nit: You could keep darwinnew around for a bit too and make it mean the same thing as just ld64.lld. Then people could update to a new build, then do the (then) no-op change of replacing darwinnew with nothing. As is, updating toolchain and changing build files have to happen in the same change, which is always a bit annoying.

But since this was advertised as an unstable flag, it's not necessary, just kind of friendly. Up to you. (If you do this, consider reverting the change to the .gn file too.)

int3 updated this revision to Diff 327145.Mar 1 2021, 9:28 AM

keep around darwinnew link for now

This revision was landed with ongoing or failed builds.Mar 1 2021, 9:30 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2021, 9:30 AM
int3 marked an inline comment as done.Mar 1 2021, 9:33 AM

Hi, I suspect thispatch might be causing the failure we're seeing in our non-mac builders (https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8853941557626730880?):

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failed Tests (2):
  Clang :: Driver/darwin-sdkroot.c
  Clang :: Driver/target-triple-deployment.c

Would you mind taking a look? Thanks.

int3 added a comment.Mar 1 2021, 10:50 AM

Thanks for the heads up. Looking

int3 added a comment.Mar 1 2021, 12:15 PM

I've reverted the clang driver changes for now in rG922de2574c17a755279358be928e5343dcf56c56.