This is an archive of the discontinued LLVM Phabricator instance.

Expand comment about how StringsToBuckets was computed, and add more entries
ClosedPublic

Authored by thakis on Jul 15 2019, 6:25 AM.

Details

Summary

The construction was explained in
https://reviews.llvm.org/D44810?id=139526#inline-391999 but reading the code
shouldn't require hunting down old reviews to understand it.

The precomputed list was missing an entry for the empty list case, and
one entry at the very end. (The current last entry is the last one where
3 * BucketCount fits in a signed int, but the reference implementation
uses unsigneds as far as I can tell, so there's room for one more entry.)

No behavior change for inputs seen in practice.

Diff Detail

Event Timeline

thakis created this revision.Jul 15 2019, 6:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2019, 6:25 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
rnk accepted this revision.Jul 15 2019, 10:19 AM

lgtm

This revision is now accepted and ready to land.Jul 15 2019, 10:19 AM
This revision was automatically updated to reflect the committed changes.