[PATCH] [libcxx] Fix __is_power2 and __next_power2. Change hashmap to handle new behavior.

[Jonathan Roelofs]

On 10/2/14 12:42 PM, Eric Fiselier wrote:
>> If this isn't/wasn't going to help out the PMF stuff in std::experimental, then I don't think that this is worth doing.
> 
> I agree, I think the best thing to do would be to simply rename the functions to avoid name collisions and then implement them separately for the PMR stuff. That way we don't have to worry about changing the performance of the hash containers.
> 
> ================
> Comment at: include/__hash_table:86
> @@ -79,2 +85,3 @@
>  {
> -    return size_t(1) << (std::numeric_limits<size_t>::digits - __clz(__n-1));
> +    return __n < 2 ? __n + (__n == 0)
> +        : size_t(1) << (std::numeric_limits<size_t>::digits - __clz(__n-1));
> ----------------
> mclow.lists wrote:
>> Looks to me that this produces:
>> 0 --> 1
>> 1 --> 1
>> 2 --> 4
>> 4 --> 8
>>
>> Is that the intended behavior? (especially the second one)
Howard mentioned something about this earlier in this thread:

>Just a word of caution:
>
>These functions were not intended to be mathematically accurate.  They were
>intended to make the unordered containers as fast as possible.  Performance
>tests (not checked in) were used to tune these functions.  Put one too many
>branches in a hot path, and you can easily destroy the performance gains that
>were intended by allowing power-of-2 bucket sizes.  I’m not saying this patch
>does this.  I’m just saying, you are in danger of doing so if you haven’t
>performance tested these changes.
>
>Also, for the purpose of the libc++ unordered containers:
>
>0 is a valid bucket count.  An upward bump of 0 buckets goes to 2.
>1 is considered not a power of 2, but it rounded up to 2.
>2 is considered prime, not a power of 2.  Thus a upward bump off bucket size 2
>goes to 5, not 4.
>
>For all other bucket sizes, it is unambiguous as to whether one is dealing
>with a prime or power of 2.
>
>These definitions are intended to promote best practices for the libc++
>unordered containers, as opposed to being mathematically accurate.
>
>Howard


> Oops, I don't think so. 
> ```
> return n < 2 ? __n + 1 
> ```
> Should fix that. 
> 
> But IDK if the change is needed. However I am still concerned that the current version exhibits undefined behaviour for `__n = 1`.
> 
> http://reviews.llvm.org/D4948
> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded