RFR: 8257815: Replace global log2 functions with efficient implementations [v10]
Claes Redestad
redestad at openjdk.java.net
Mon Dec 14 13:08:04 UTC 2020
On Mon, 14 Dec 2020 12:54:09 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> Claes Redestad has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 26 additional commits since the last revision:
>>
>> - With JDK-8257985 fixed, use count_trailing_zeros to implement log2i_exact
>> - Merge branch 'master' into log2_template
>> - Merge branch 'master' of https://github.com/openjdk/jdk into log2_template
>> - More review comments adressed
>> - Issue with using count_trailing_zeros with 64-bit values on 32-bit, revert and add TODO
>> - Merge branch 'master' into log2_template
>> - Renaming, adress review comments
>> - x->value
>> - Rename log2i ilogi and make it only accept value > 0, rename log2i_allow_zero log2i_graceful, accepting any value
>> - Rename exact_log2i exact_ilog2, make it stricter
>> - ... and 16 more: https://git.openjdk.java.net/jdk/compare/176978c5...42b75337
>
> src/hotspot/share/utilities/powerOfTwo.hpp line 78:
>
>> 76: assert(is_power_of_2(value),
>> 77: "value must be a power of 2: " UINT64_FORMAT,
>> 78: (uint64_t)static_cast<typename std::make_unsigned<T>::type>(value));
>
> This looks strange and complicated for something that ought to be straightforward. There was a comment in conversation suggesting this is working around some problem with some test. Please explain, because I'd really like this to go back to just a cast to uint64_t if at all possible. Sorry I didn't bring this up earlier.
For signed int 0x80000000 casting to uint64_t will change the value to 0xFFFFFFFF80000000, which is avoided by making the value unsigned before casting (0x0000000080000000). After we changed the precondition to require value to be positive this is only an issue for what the assert prints in this edge case, so I agree we can simplify it.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1663
More information about the hotspot-dev
mailing list