[PATCH] D60297: [COFF] Pack Name in Symbol as is done in ELF

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/COFF/Symbols.h:111
+  // Symbol name length. Assume symbol lengths fit in a 32-bit integer.
+  unsigned NameLen;
+
----------------
aganea wrote:
> aganea wrote:
> > ruiu wrote:
> > > ruiu wrote:
> > > > I'd use uint32_t to match the comment.
> > > We have the same hack for ELF, and we name these variables NameSize and NameData.
> > You could save more by packing this with the bitfield above. ‘SymbolKind’ doesn’t need more than 4 bits, and that would free 21 bits for ‘NameLen’. In practical terms, do we really expect symbol lengths > 2^21 chars? Worst case, there could be a slower/higher mem. codepath for off-the-limits case?
> Forget what I said, it’ll be aligned on 64-bits because of the pointer that follows.
If uint32_t happens to be a long, will this prevent packing with the bitfields in the ms abi? maybe unsigned is better for that reason?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60297/new/

https://reviews.llvm.org/D60297