This is an archive of the discontinued LLVM Phabricator instance.

MachObjectWriter: optimize the string table for common suffices
ClosedPublic

Authored by hans on Oct 5 2014, 6:03 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

hans updated this revision to Diff 14438.Oct 5 2014, 6:03 PM
hans retitled this revision from to MachObjectWriter: optimize the string table for common suffices.
hans updated this object.
hans edited the test plan for this revision. (Show Details)
hans added a reviewer: majnemer.
hans added subscribers: Unknown Object (MLST), hansw.
hans added inline comments.
test/MC/MachO/variable-exprs.s
208 ↗(On Diff #14438)

This is the only part where I'm not sure what's going on :/

majnemer accepted this revision.Oct 5 2014, 6:37 PM
majnemer edited edge metadata.

LGTM

lib/MC/StringTableBuilder.cpp
72 ↗(On Diff #14438)

Would it make sense to have an else clause that asserted the kind was ELF? Maybe use a fully covered switch? My thinking is that it would make it easier to catch potential bugs if we ever added a new object file format.

test/MC/MachO/variable-exprs.s
208 ↗(On Diff #14438)

Obligatory arcane Mach-O infodump:

If the type is N_INDR then the symbol is defined to be the same as another symbol.
In this case the n_value field is an index into the string table of the other symbol's name.

d2 is defined to be the same as d. d has a n_strx of 7. This means that d2 should have an n_value of 7 as well.

This revision is now accepted and ready to land.Oct 5 2014, 6:37 PM
hans added inline comments.Oct 6 2014, 10:15 AM
lib/MC/StringTableBuilder.cpp
72 ↗(On Diff #14438)

Sounds good to me. I've turned it into a switch, and did the same above.

test/MC/MachO/variable-exprs.s
208 ↗(On Diff #14438)

Thanks for digging that out! Now it makes sense :)

hans closed this revision.Oct 6 2014, 10:15 AM
hans updated this revision to Diff 14461.

Closed by commit rL219126 (authored by @hans).