RFR: 8287215: Add method to scan bitmap backwards [v4]

[Thomas Schatzl]

On Mon, 30 May 2022 11:36:38 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Thomas Schatzl has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Make r_index exclusive
>
> src/hotspot/share/utilities/bitMap.inline.hpp line 233:
> 
>> 231: template<BitMap::bm_word_t flip, bool aligned_left>
>> 232: inline BitMap::idx_t BitMap::get_prev_bit_impl(idx_t l_index, idx_t r_index) const {
>> 233:   STATIC_ASSERT(flip == find_ones_flip || flip == find_zeros_flip);
> 
> We have `static_assert` now, which requires a message but gives easier to understand errors.

Fixed, but I'm not sure it is better now.

> src/hotspot/share/utilities/bitMap.inline.hpp line 270:
> 
>> 268:         return result;
>> 269:       }
>> 270:       // Result is beyond range bound.
> 
> Maybe add "return NotFound" for consistency with later comment?

I added a comment that this intentionally falls through to the outer `return NotFound`.

> src/hotspot/share/utilities/bitMap.inline.hpp line 278:
> 
>> 276:         cword = map(index) ^ flip;
>> 277:         if (cword != 0) {
>> 278:           idx_t result = bit_index(index + 1) - count_leading_zeros(cword) - 1;
> 
> I think this calculation would be clearer as
> 
> bit_index(index) + (BitsPerWord - 1) - count_leading_zeros(cword)
> 
> getting the position of the last bit in the current word directly and counting backward from there.
> 
> It's ultimately the same calculation, but I found the suggested alternative more obvious and easier to reason about.  For example, can `bit_index(index + 1)` overflow?  It can't, but I had to think about it.

Either is fine. Fixed.

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

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