This is an archive of the discontinued LLVM Phabricator instance.

[lld][WebAssembly] Add libcall symbols to the link when LTO is being used.
ClosedPublic

Authored by sbc100 on Dec 19 2019, 5:27 PM.

Details

Summary

This code is copied almost verbatim from the equivalent change to the
ELF linker:

The upshot is that libraries containing libcall (such as compiler-rt
and libc) can be compiled with LTO.

Fixes PR41384

Diff Detail

Event Timeline

sbc100 created this revision.Dec 19 2019, 5:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 19 2019, 5:27 PM

ping.. should be uncontroversial since this code its basically identical to ELF/COFF backends,.

sbc100 added a reviewer: pcc.Dec 20 2019, 12:41 PM

Adding pcc who wrote the original code.

sbc100 edited the summary of this revision. (Show Details)Dec 20 2019, 1:07 PM
pcc added inline comments.Dec 20 2019, 1:28 PM
lld/wasm/Driver.cpp
447

Do you really need the isBitcode check in the wasm linker? We needed it in the ELF linker because of some badly behaving libgcc functions, but maybe wasm doesn't have anything like that?

sbc100 marked an inline comment as done.Dec 20 2019, 2:10 PM
sbc100 added inline comments.
lld/wasm/Driver.cpp
447

But then wouldn't we end up fetching native object from libraries unnecessarily?

Without this check, if (for example) compiler-rt and libc are compiled as native object files, wouldn't we be loading in a lot of objects that we might never need.

friendly ping.

pcc added inline comments.Jan 9 2020, 11:33 AM
lld/wasm/Driver.cpp
447

Sure, but does that matter? If the user passes --gc-sections (meaning that they care about binary size) that shouldn't make a difference to the final output file, I would have thought. Yes, there would be some overhead from loading the object files but maybe it's not enough to worry about. I think either way would be fine with me though.

765

This comment talks about the issue that we hit on ARM and doesn't apply to wasm. I guess you could talk about performance or file size here instead.

sbc100 updated this revision to Diff 237145.Jan 9 2020, 11:53 AM
sbc100 edited the summary of this revision. (Show Details)
  • add comment
sbc100 marked an inline comment as done.Jan 9 2020, 11:54 AM
pcc accepted this revision.Jan 9 2020, 4:54 PM

LGTM

lld/wasm/Driver.cpp
763

the the -> the

This revision is now accepted and ready to land.Jan 9 2020, 4:54 PM
This revision was automatically updated to reflect the committed changes.