RFR: 8257985: count_trailing_zeros doesn't handle 64-bit values on 32-bit JVM [v2]

[Claes Redestad]

On Fri, 11 Dec 2020 12:16:35 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Claes Redestad has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix Windows 32-bit variant, improve commentary
>
> src/hotspot/share/utilities/count_trailing_zeros.hpp line 98:
> 
>> 96:     if (index == 0) {
>> 97:       _BitScanForward(&index, (uint32_t)(v >> 32));
>> 98:       if (index > 0) {
> 
> Shouldn't this check be superfluous if the code previously asserted v != 0, and we know that the lower 32 bits are zero?
> 
> Also the MSDN documentation does not state that `index == 0` if no bit has been found, but the return value of the method is 0. I.e. shouldn't the implementation be something like:
> 
> if (_BitScanForward(&index, (uint32_t)v) == 0) { // no bit found? If so, try the upper dword. Otherwise index already contains the result
>   _BitScanForward(&index, v >> 32); // no need for the uint32_t cast. >> is well defined on unsigned longs.
>   // one could assert that the result of this invocation is non-zero, but it should be 1 if v != 0.
>   index += 32;
> }
> return index;

Yes, I misread the MSDN documentation and it appears index is undefined if there are no bits and we should rely on the return value.

Omitting the cast on `v >> 32` seems fine, but the comment is a bit off since `v` can be a signed type here.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1747