[libcxx] r275114 - Don't compute modulus of hash if it is smaller than the bucket count.

Chandler Carruth via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 18 00:18:31 PDT 2016


On Sun, Jul 17, 2016 at 1:54 PM Arthur O'Dwyer <arthur.j.odwyer at gmail.com>
wrote:

> Given that this patch is basically Chandler's talk from CppCon 2015 (
> https://www.youtube.com/watch?v=nXaxk27zwlk), I'm surprised that the
> commit message isn't explicitly mentioning that; and surprised that
> Chandler himself isn't weighing in on either the "this is a good idea" or
> "this is a bad idea" side.
>

FWIW, I don't follow all of these commits that closely. ;] I didn't know
about this until you CC'ed me.


>
> IMHO, if replacing "%" with "fastmod" in general-purpose code like this
> were a good idea,
> (A) libc++ should introduce a helper function __fastmod(m,n) for the
> purpose, not repeat the same patch everywhere there's currently a "%"
> operator; and/or
> (B) someone with authority over the Clang x86 backend
> (*cough*Chandler*cough*) should look into improving the codegen for "%" by
> auto-detecting when it might make sense to use this heuristic.
>

FWIW, I'm much more interested in the compiler doing this than in source
code doing this if there is a general desire to do this kind of thing.

However, In the specific case of hash tables I can't say I really care.
That is because I believe it is a serious mistake to use non-mixed hash
functions and prime sized tables where the modulo is a significant cost.
Instead, I would very much like to see C++ move toward hash functions which
always mix well (pass avalanche tests, etc) and power-of-two size tables
where this becomes a mask rather than a modulo operation, and thus
dramatically faster.

My two cents, should it matter to folks in the future. None of this I think
changes the reasonable short-term goals Eric is working on -- a change of
this magnitude would be quite separate.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160718/a1ae3076/attachment.html>


More information about the cfe-commits mailing list