[PATCH] D81296: [PDB] Defer public serialization until PDB writing

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a comment.

Thanks for doing this, that's a nice saving :-) A few minor things:



================
Comment at: llvm/include/llvm/DebugInfo/PDB/Native/GSIStreamBuilder.h:122
+  // PublicSymFlags.
+  uint16_t Flags : 3;
 
----------------
`PublicSymFlags` needs 4 bits, you could take one from `BucketIdx` below since you don't need more than 12 bits for that - you're doing `% IPHR_HASH` below.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:198
+  // bucket start offsets.
+  uint32_t BucketStarts[IPHR_HASH];
+  memset(&BucketStarts[0], 0, sizeof(BucketStarts));
----------------
You could also do a one-liner `uint32_t BucketStarts[IPHR_HASH] = {0};`


================
Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:203
+  uint32_t Sum = 0;
+  for (uint32_t &B : BucketStarts) {
+    uint32_t Size = B;
----------------
This also could be:
```
for (unsigned I = 1; I < IPHR_HASH; ++I)
  BucketStarts[I] += BucketStarts[I-1];
BucketStarts[0] = 0;
```


================
Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:213
+  HashRecords.resize(Records.size());
+  uint32_t BucketCursors[IPHR_HASH];
+  memcpy(BucketCursors, BucketStarts, sizeof(BucketCursors));
----------------
Same as above: `uint32_t BucketCursors[IPHR_HASH] = {0};`


================
Comment at: llvm/lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp:243
-
-      // Calculate what the offset of the first hash record in the chain would
-      // be if it were inflated to contain 32-bit pointers. On a 32-bit system,
----------------
This comment got lost, could you please re-add it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81296