This is an archive of the discontinued LLVM Phabricator instance.

caseFoldingDjbHash: simplify and make the US-ASCII fast path faster
ClosedPublic

Authored by MaskRay on Apr 26 2019, 2:47 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

MaskRay created this revision.Apr 26 2019, 2:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 26 2019, 2:47 AM

I measured 1.8x speedup on the corpus of readelf -s clang | awk '{print $8}' (purely US-ASCII)..

labath accepted this revision.Apr 26 2019, 3:13 AM

Interesting. I take it the new code pipelines/unrolls better because it has a single branchless loop.

In terms of readability, I'd still try to split this into two functions, assuming that doesn't have a negative performance impact (which I'm hoping it doesn't). I'm thinking of something like

if (Optional<uint32_t> Result = fastCaseFoldingDjbHash(Buffer, H))
  return *Result;

// slow stuff...

Other than that, this looks good to me. Thanks for improving this.

This revision is now accepted and ready to land.Apr 26 2019, 3:13 AM
MaskRay updated this revision to Diff 196827.Apr 26 2019, 3:42 AM

split into two functions

if (Optional<uint32_t> Result = fastCaseFoldingDjbHash(Buffer, H))
  return *Result;

doesn't have a negative performance impact. Switched to that form.

This revision was automatically updated to reflect the committed changes.
aprantl added inline comments.Apr 26 2019, 2:53 PM
llvm/trunk/lib/Support/DJB.cpp
61

This should be AllASCII according to the LLVM style?

MaskRay marked 2 inline comments as done.Apr 27 2019, 8:30 AM
MaskRay added inline comments.
llvm/trunk/lib/Support/DJB.cpp
61

Fixed!