This is an archive of the discontinued LLVM Phabricator instance.

Object: Shrink the size of irsymtab::Symbol by a word. NFCI.
ClosedPublic

Authored by pcc on Apr 13 2017, 7:56 PM.

Details

Summary

Instead of storing an UncommonIndex on the Symbol, use a flag bit to store
whether the Symbol has an Uncommon. This shrinks Chromium's .bc files (after
D32061) by about 1%.

Event Timeline

pcc created this revision.Apr 13 2017, 7:56 PM
tejohnson added inline comments.Apr 17 2017, 12:29 PM
llvm/include/llvm/Object/IRSymtab.h
288

Should this be Uncommons.end() instead of nullptr? Otherwise, will UncI ever be nullptr allowing the SymbolRef::operator== to detect the end of the symbol_range? I see where we increment UncI, but not how it could be nullptr at the end of the Uncommons range.

llvm/lib/Object/IRSymtab.cpp
37

Is the restructuring in this file related to the change to set the new flag? I couldn't figure out how. It looks like a good change, but if unrelated can you commit separately?

pcc added inline comments.Apr 17 2017, 1:29 PM
llvm/include/llvm/Object/IRSymtab.h
288

We only use SymI for iterator comparisons, so it doesn't matter what value UncI has at the end of the range.

llvm/lib/Object/IRSymtab.cpp
37

I made this change to simplify setting a correct value for UncBegin in the storage::Module, which would be more difficult if we enumerated modules separately from symbols.

tejohnson accepted this revision.Apr 17 2017, 3:42 PM

LGTM with one suggestion below.

llvm/include/llvm/Object/IRSymtab.h
288

Ok, missed that. Still, it seems odd not just to use Uncommons.end() here, instead of the less-meaningful nullptr. Suggest changing if that doesn't hurt anything.

llvm/lib/Object/IRSymtab.cpp
37

Got it - because Uncommons wouldn't be updated after each call to addModule.

This revision is now accepted and ready to land.Apr 17 2017, 3:42 PM
pcc added inline comments.Apr 17 2017, 4:25 PM
llvm/include/llvm/Object/IRSymtab.h
288

I think that would be a little confusing. If I were reading this code and I saw Uncommons.end() here I would draw the conclusion that the pointer value in the end iterator is probably being used somehow, which would lead me to believe that there may be a bug in module_symbols() because it is passing nullptr. Using nullptr in both places makes it clear that the value is unimportant.

tejohnson added inline comments.Apr 17 2017, 4:29 PM
llvm/include/llvm/Object/IRSymtab.h
288

I guess it isn't straightforward to get the end iterator in the module_symbols case. Fair enough.

This revision was automatically updated to reflect the committed changes.