This is an archive of the discontinued LLVM Phabricator instance.

[Driver][Fuchsia] Add default linker flags
ClosedPublic

Authored by abrachet on Aug 22 2022, 3:00 PM.

Diff Detail

Event Timeline

abrachet created this revision.Aug 22 2022, 3:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2022, 3:00 PM
abrachet requested review of this revision.Aug 22 2022, 3:00 PM
mcgrathr accepted this revision.Aug 22 2022, 4:31 PM
mcgrathr added a reviewer: phosek.
mcgrathr added inline comments.
clang/lib/Driver/ToolChains/Fuchsia.cpp
58

There's a test for lld right below, so you could also just put these in an else block of that.
But I'll leave it to Petr to say which he prefers.

There are only test cases that use -fuse-ld=lld explicitly, which seems odd. It's not so odd that we haven't bothered to test the non-lld behavior before (though ideally we should). But it's a bit odd that we pass it explicitly when it's the default.

phosek accepted this revision.Aug 22 2022, 6:11 PM

LGTM

clang/lib/Driver/ToolChains/Fuchsia.cpp
58

I'd prefer passing these in the else block but either way is fine with me.

This revision is now accepted and ready to land.Aug 22 2022, 6:11 PM
abrachet updated this revision to Diff 454924.Aug 23 2022, 12:21 PM

Probably worth looking at this again before I submit

phosek accepted this revision.Aug 23 2022, 2:35 PM

LGTM

This revision was landed with ongoing or failed builds.Sep 30 2022, 7:07 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2022, 7:07 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
thakis added a subscriber: thakis.Sep 30 2022, 7:41 AM

This breaks tests on non-linux:

http://45.33.8.238/macm1/45619/step_7.txt
http://45.33.8.238/win/67193/step_7.txt

Please take a look and revert for now if it takes a while to fix.