[PATCH] D44810: [PDB] Make LLD PDBs look a little more like Microsoft PDBs

[David Majnemer via Phabricator via llvm-commits]

majnemer added inline comments.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/PDBStringTableBuilder.cpp:41-47
+  //   int BucketCount = 1;
+  //   int StringCount = 0;
+  //   while (true) {
+  //     ++StringCount;
+  //     if (BucketCount * 3 / 4 < StringCount)
+  //       BucketCount = BucketCount * 3 / 2 + 1;
+  //   }
----------------
zturner wrote:
> majnemer wrote:
> > This pseudo-code infinite loops. What's the termination condition?
> The body of the loop is basically the "insert" function of the hash table, so it's trying to illustrate that on each insert, they increment the count, and if the load factor goes over 75% they multiply the number of buckets by 1.5 and rehash.
> 
> If there's a way to write the pseudocode that's more clear I'm happy to change it.
> 
> What about this:
> 
> ```
> // Initial conditions
> int BucketCount = 1;
> int StringCount = 0;
> 
> // Called each time a new string is inserted.
> void onStringInserted() {
>   ++StringCount;
>   if (BucketCount * 3 / 4 < StringCount)
>     BucketCount = BucketCount * 3 / 2 + 1;
> }
> ```
SGTM.

However, I think the sign has some interesting implications.

If StringCount was 49 (with BucketCount 1034394550), this code will overflow:
1034394550 * 3 is -1191783646 which will be less than StringCount (now 50).

Your version will end up computing the mathematically correct result of 1551591826.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/PDBStringTableBuilder.cpp:48-49
+  //   }
+  // This was determined emperically to be identical to https://oeis.org/A006999
+  // which offers the closed form.
+  //
----------------
a006999 53 is 3491081611
a006999 54 is 5236622417, this number is too big to fit in a uint32_t. This would end up computing (uint32_t)5236622417 which is 941655121.


https://reviews.llvm.org/D44810