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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
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)};
 }
----------------
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.


================
Comment at: llvm/lib/Object/IRSymtab.cpp:37
   DenseMap<const Comdat *, unsigned> ComdatMap;
-  ModuleSymbolTable Msymtab;
-  SmallPtrSet<GlobalValue *, 8> Used;
----------------
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.


https://reviews.llvm.org/D32070