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

[Zachary Turner via Phabricator via llvm-commits]

zturner 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;
+  //   }
----------------
majnemer wrote:
> 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.
I'll have to check with the MS source code tomorrow and see if theirs overflows.  Perhaps they're using a uint64 and I missed that.


https://reviews.llvm.org/D44810