[PATCH] D32070: Object: Shrink the size of irsymtab::Symbol by a word. NFCI.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM with one suggestion below.



================
Comment at: llvm/include/llvm/Object/IRSymtab.h:288
+  return {SymbolRef(Symbols.begin(), Symbols.end(), Uncommons.begin(), this),
+          SymbolRef(Symbols.end(), Symbols.end(), nullptr, this)};
 }
----------------
pcc wrote:
> tejohnson wrote:
> > 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.
> We only use `SymI` for iterator comparisons, so it doesn't matter what value `UncI` has at the end of the range.
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.


================
Comment at: llvm/lib/Object/IRSymtab.cpp:37
   DenseMap<const Comdat *, unsigned> ComdatMap;
-  ModuleSymbolTable Msymtab;
-  SmallPtrSet<GlobalValue *, 8> Used;
----------------
pcc wrote:
> tejohnson wrote:
> > 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?
> 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.
Got it - because Uncommons wouldn't be updated after each call to addModule.


https://reviews.llvm.org/D32070