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

[Thomas Schatzl]

On Fri, 11 Dec 2020 11:24:08 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Re-implement count_trailing_zeros as a template function similar to count_leading_zeros, adding support for 8 and 16 bit types as well as for 64-bit types on 32-bit builds, which is currently not supported. This prohibits implementing the log2i_exact proposed by #1663 using count_trailing_zeros, as suggested during review.
>> 
>> Compared to count_leading_zeros the implementation can be slightly simpler since subword specializations can simply use the internal 32-bit variant.
>> 
>> Windows doesn't define _BitScanForward64 on 32-bit targets, so the 32-bit variant for handling 64-bit values becomes more convoluted.
>
> Claes Redestad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix Windows 32-bit variant, improve commentary

Changes requested by tschatzl (Reviewer).

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;

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

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