Page MenuHomePhabricator

[lld-macho] Have dead-stripping work with literal sections
ClosedPublic

Authored by int3 on Jun 9 2021, 10:25 AM.

Details

Summary

Literal sections are not atomically live or dead. Rather,
liveness is tracked for each individual literal they contain. CStrings
have their liveness tracked via a live bit in StringPiece, and
fixed-width literals have theirs tracked via a BitVector.

The live-marking code now needs to track the offset within each section
that is to be marked live, in order to identify the literal at that
particular offset.

Numbers for linking chromium_framework on my 3.2 GHz 16-Core Intel Xeon W
with both -dead_strip and --deduplicate-literals, with and without this diff
applied:

    N           Min           Max        Median           Avg        Stddev
x  20          4.32          4.44         4.375         4.372    0.03105174
+  20           4.3          4.39          4.36        4.3595   0.023277502
No difference proven at 95.0% confidence

This gives us size savings of about 0.4%.

Diff Detail

Event Timeline

int3 created this revision.Jun 9 2021, 10:25 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: jfb. · View Herald Transcript
int3 requested review of this revision.Jun 9 2021, 10:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2021, 10:25 AM
int3 added a comment.Jun 9 2021, 10:35 AM

Running some benchmarks locally, will update commit message in a bit

int3 edited the summary of this revision. (Show Details)Jun 9 2021, 11:10 AM
int3 edited the summary of this revision. (Show Details)
oontvoo added a subscriber: oontvoo.Jun 9 2021, 8:08 PM
oontvoo added inline comments.
lld/MachO/InputSection.h
123

IMHO, 31 is a bit weird. (It's only 1 bit difference!)

int3 added inline comments.Jun 9 2021, 8:49 PM
lld/MachO/InputSection.h
123

1 extra bit here makes the struct take up 1 extra word :)

This was taken from LLD-ELF; I have yet to profile it myself, but it seemed like a reasonable idea

craig.topper added inline comments.
lld/MachO/InputSection.h
123

Should live also be uint32_t? If i recall, MSVC needs the types of bitfields to be the same in order to merge the storage.

tschuett added inline comments.
lld/MachO/InputSection.h
123

Could you add a static_assert for proof and documentation?

int3 added inline comments.Jun 9 2021, 10:02 PM
lld/MachO/InputSection.h
123

ohh, so that's why LLD-ELF uses a uint32_t. Yeah, I'll add a static_assert.

int3 updated this revision to Diff 351064.Jun 9 2021, 10:10 PM
int3 marked 3 inline comments as done.

static_assert

thakis accepted this revision.Jun 10 2021, 5:26 AM
thakis added a subscriber: thakis.

Nice!

This revision is now accepted and ready to land.Jun 10 2021, 5:26 AM
This revision was landed with ongoing or failed builds.Jun 11 2021, 4:50 PM
This revision was automatically updated to reflect the committed changes.
lei added a subscriber: lei.EditedJun 14 2021, 9:04 AM

Hi,

The changes in the following commits have been causing failures in our bot, https://lab.llvm.org/buildbot/#/workers/50, since last Friday:

All 5 patches seem to be related and pushed at the same time. Please provide a fix to get the bots green or we will have to pull all 5 patches by EOD EDT.

Thank-you