RFR: 8183574: Unify the is_power_of_2 functions

[Stefan Karlsson]

On 2020-02-13 22:54, Kim Barrett wrote:
>> On Feb 13, 2020, at 9:47 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>
>> Hi all,
>>
>> Please review this patch to consolidate all is_power_of_2 functions into one template function and place it in the existing powerOfTwo.hpp file.
>>
>> To resolve a circular dependency between globalDefinitions.hpp and powerOfTwo.hpp, I also moved exact_log2 and exact_log2_long. These functions take a power-of-two value and returns the log2 value, so I think it makes sense to move them to powerOfTwo.hpp.
>>
>> https://cr.openjdk.java.net/~stefank/8183574/webrev.01
>> https://bugs.openjdk.java.net/browse/JDK-8183574
>>
>> Tested with tier1,2,3
>> Also built locally with {fastdebug,release}x{minimal,zero,aarch64,ppc,s390,arm32,shenandoah}
>>
>> Thanks,
>> StefanK
> That was a lot(!) of files to click through.
>
> Looks good.

Thanks.

>
> ------------------------------------------------------------------------------
> src/hotspot/share/code/vtableStubs.cpp
>   132     assert(is_power_of_2(int(N)), "N must be a power of 2");
>
> Conversion of N to int is because it's defined via enum (because
> that's what we used to do).  Better would be to no longer define N
> that way.  (Also the related mask.)

I found references that using enums here would be valid in C++11.

>
> This could be a followup cleanup if that's more convenient.  And I
> don't need a new webrev if you change this now.

OK. I'd prefer to leave this as is, because it's the minimal patch 
needed to get this to compile.

>
> ------------------------------------------------------------------------------
>
> I also noticed some probably unnecessary casts here and there.  I
> think those are better dealt with separately (perhaps as the code is
> touched for other reasons), rather than adding clutter to this change.

OK.

Thanks for the review,
StefanK

>
>