This is an archive of the discontinued LLVM Phabricator instance.

Tweak SimpleFastHash
ClosedPublic

Authored by aarongreen on Jan 12 2021, 9:27 AM.

Details

Summary

This change adds a SimpleFastHash64 variant of SimpleFastHash which allows call sites to specify a starting value and get a 64 bit hash in return. This allows a hash to be "resumed" with more data.

A later patch needs this to be able to hash a sequence of module-relative values one at a time, rather than just a region a memory.

Diff Detail

Event Timeline

aarongreen requested review of this revision.Jan 12 2021, 9:27 AM
aarongreen created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2021, 9:27 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

Can you explain in the summary / commit message why this is needed? How do you intend to use it?

aarongreen edited the summary of this revision. (Show Details)Feb 3 2021, 11:01 AM

Can you explain in the summary / commit message why this is needed? How do you intend to use it?

Done. Basically, we are going to want the hash of a sequence of position-independent / DSO-relative PCs rather than just of the whole PC table in one go.

morehouse added inline comments.Feb 3 2021, 2:29 PM
compiler-rt/lib/fuzzer/FuzzerUtil.cpp
243

Why do we need uint64_t instead of size_t? Using just one or the other seems simpler.

aarongreen updated this revision to Diff 321590.Feb 4 2021, 3:54 PM
aarongreen marked an inline comment as done.

Combined two versions of SimpleFastHash into one.

morehouse added inline comments.Feb 5 2021, 8:19 AM
compiler-rt/lib/fuzzer/FuzzerUtil.h
93

It seems a little silly to have the same value returned and written to *Hash? WDYT about simplifying to:

uint64_t SimpleFastHash(const void *Data, size_t Size, uint64_t Hash = 0)
aarongreen added inline comments.Feb 5 2021, 8:22 AM
compiler-rt/lib/fuzzer/FuzzerUtil.h
93

That's fine. I have places in later changes like:

uint64_t Hash = Initial;
for (...)
  SimpleFastHash(..., &Hash);

but I can changes those.

aarongreen updated this revision to Diff 321771.Feb 5 2021, 8:40 AM
This revision is now accepted and ready to land.Feb 5 2021, 9:25 AM

See comments on D94512 and D94514. This revision no longer descends from D94508 or D94509 as we no longer need fuzzer::ModuleRelativeValues or the changes to FuzzerFork.cpp.

aarongreen edited the summary of this revision. (Show Details)Mar 31 2021, 11:40 AM
This revision was landed with ongoing or failed builds.Apr 1 2021, 11:26 PM
Closed by commit rG0889181625bb: Tweak SimpleFastHash (authored by aarongreen, committed by charco). · Explain Why
This revision was automatically updated to reflect the committed changes.