From 2d0d057d6691d4abe4ca1ef44b29f03043323b67 Mon Sep 17 00:00:00 2001 From: Kim Barrett Date: Mon, 20 Mar 2023 19:23:38 +0000 Subject: [PATCH] 8304016: Add BitMap find_last suite of functions Reviewed-by: stefank, aboldtch --- src/hotspot/share/utilities/bitMap.hpp | 34 ++++- src/hotspot/share/utilities/bitMap.inline.hpp | 139 +++++++++++++----- .../gtest/utilities/test_bitMap_search.cpp | 95 +++++++++--- 3 files changed, 204 insertions(+), 64 deletions(-) diff --git a/src/hotspot/share/utilities/bitMap.hpp b/src/hotspot/share/utilities/bitMap.hpp index a6e62bc16ba..8a55d410c3e 100644 --- a/src/hotspot/share/utilities/bitMap.hpp +++ b/src/hotspot/share/utilities/bitMap.hpp @@ -98,10 +98,19 @@ class BitMap { // - flip designates whether searching for 1s or 0s. Must be one of // find_{zeros,ones}_flip. // - aligned_right is true if end is a priori on a bm_word_t boundary. + // - returns end if not found. template inline idx_t find_first_bit_impl(idx_t beg, idx_t end) const; - // Values for find_first_bit_impl flip parameter. + // Helper for find_last_{set,clear}_bit variants. + // - flip designates whether searching for 1s or 0s. Must be one of + // find_{zeros,ones}_flip. + // - aligned_left is true if beg is a priori on a bm_word_t boundary. + // - returns end if not found. + template + inline idx_t find_last_bit_impl(idx_t beg, idx_t end) const; + + // Values for find_{first,last}_bit_impl flip parameter. static const bm_word_t find_ones_flip = 0; static const bm_word_t find_zeros_flip = ~(bm_word_t)0; @@ -287,9 +296,9 @@ class BitMap { return iterate(cl, 0, size()); } - // Looking for 1's and 0's at indices equal to or greater than "beg", - // stopping if none has been found before "end", and returning - // "end" (which must be at most "size") in that case. + // Return the index of the first set (or clear) bit in the range [beg, end), + // or end if none found. + // precondition: beg and end form a valid range for the bitmap. idx_t find_first_set_bit(idx_t beg, idx_t end) const; idx_t find_first_clear_bit(idx_t beg, idx_t end) const; @@ -304,6 +313,23 @@ class BitMap { // aligned to bitsizeof(bm_word_t). idx_t find_first_set_bit_aligned_right(idx_t beg, idx_t end) const; + // Return the index of the last set (or clear) bit in the range [beg, end), + // or end if none found. + // precondition: beg and end form a valid range for the bitmap. + idx_t find_last_set_bit(idx_t beg, idx_t end) const; + idx_t find_last_clear_bit(idx_t beg, idx_t end) const; + + idx_t find_last_set_bit(idx_t beg) const { + return find_last_set_bit(beg, size()); + } + idx_t find_last_clear_bit(idx_t beg) const { + return find_last_clear_bit(beg, size()); + } + + // Like "find_last_set_bit", except requires that "beg" is + // aligned to bitsizeof(bm_word_t). + idx_t find_last_set_bit_aligned_left(idx_t beg, idx_t end) const; + // Returns the number of bits set in the bitmap. idx_t count_one_bits() const; diff --git a/src/hotspot/share/utilities/bitMap.inline.hpp b/src/hotspot/share/utilities/bitMap.inline.hpp index 76e72117d87..41b6cc0c0e6 100644 --- a/src/hotspot/share/utilities/bitMap.inline.hpp +++ b/src/hotspot/share/utilities/bitMap.inline.hpp @@ -30,6 +30,7 @@ #include "runtime/atomic.hpp" #include "utilities/align.hpp" #include "utilities/count_trailing_zeros.hpp" +#include "utilities/powerOfTwo.hpp" inline void BitMap::set_bit(idx_t bit) { verify_index(bit); @@ -165,49 +166,51 @@ inline void BitMap::par_clear_range(idx_t beg, idx_t end, RangeSizeHint hint) { } } +// General notes regarding find_{first,last}_bit_impl. +// +// The first (last) word often contains an interesting bit, either due to +// density or because of features of the calling algorithm. So it's important +// to examine that word with a minimum of fuss, minimizing setup time for +// additional words that will be wasted if the that word is indeed +// interesting. +// +// The first (last) bit is similarly often interesting. When it matters +// (density or features of the calling algorithm make it likely that bit is +// set), going straight to counting bits compares poorly to examining that bit +// first; the counting operations can be relatively expensive, plus there is +// the additional range check (unless aligned). But when that bit isn't set, +// the cost of having tested for it is relatively small compared to the rest +// of the search. +// +// The benefit from aligned_right being true is relatively small. It saves an +// operation in the setup of the word search loop. It also eliminates the +// range check on the final result. However, callers often have a comparison +// with end, and inlining may allow the two comparisons to be combined. It is +// important when !aligned_right that return paths either return end or a +// value dominated by a comparison with end. aligned_right is still helpful +// when the caller doesn't have a range check because features of the calling +// algorithm guarantee an interesting bit will be present. +// +// The benefit from aligned_left is even smaller, as there is no savings in +// the setup of the word search loop. + template inline BitMap::idx_t BitMap::find_first_bit_impl(idx_t beg, idx_t end) const { STATIC_ASSERT(flip == find_ones_flip || flip == find_zeros_flip); verify_range(beg, end); assert(!aligned_right || is_aligned(end, BitsPerWord), "end not aligned"); - // The first word often contains an interesting bit, either due to - // density or because of features of the calling algorithm. So it's - // important to examine that first word with a minimum of fuss, - // minimizing setup time for later words that will be wasted if the - // first word is indeed interesting. - - // The benefit from aligned_right being true is relatively small. - // It saves an operation in the setup for the word search loop. - // It also eliminates the range check on the final result. - // However, callers often have a comparison with end, and - // inlining often allows the two comparisons to be combined; it is - // important when !aligned_right that return paths either return - // end or a value dominated by a comparison with end. - // aligned_right is still helpful when the caller doesn't have a - // range check because features of the calling algorithm guarantee - // an interesting bit will be present. - if (beg < end) { // Get the word containing beg, and shift out low bits. idx_t word_index = to_words_align_down(beg); bm_word_t cword = flipped_word(word_index, flip) >> bit_in_word(beg); - if ((cword & 1) != 0) { - // The first bit is similarly often interesting. When it matters - // (density or features of the calling algorithm make it likely - // the first bit is set), going straight to the next clause compares - // poorly with doing this check first; count_trailing_zeros can be - // relatively expensive, plus there is the additional range check. - // But when the first bit isn't set, the cost of having tested for - // it is relatively small compared to the rest of the search. + if ((cword & 1) != 0) { // Test the beg bit. return beg; - } else if (cword != 0) { - // Flipped and shifted first word is non-zero. - idx_t result = beg + count_trailing_zeros(cword); - if (aligned_right || (result < end)) return result; - // Result is beyond range bound; return end. - } else { - // Flipped and shifted first word is zero. Word search through + } + // Position of bit0 of cword in the bitmap. Initially for shifted first word. + idx_t cword_pos = beg; + if (cword == 0) { // Test other bits in the first word. + // First word had no interesting bits. Word search through // aligned up end for a non-zero flipped word. idx_t word_limit = aligned_right ? to_words_align_down(end) // Minuscule savings when aligned. @@ -215,14 +218,61 @@ inline BitMap::idx_t BitMap::find_first_bit_impl(idx_t beg, idx_t end) const { while (++word_index < word_limit) { cword = flipped_word(word_index, flip); if (cword != 0) { - idx_t result = bit_index(word_index) + count_trailing_zeros(cword); - if (aligned_right || (result < end)) return result; - // Result is beyond range bound; return end. - assert((word_index + 1) == word_limit, "invariant"); + // Update for found non-zero word, and join common tail to compute + // result from cword_pos and non-zero cword. + cword_pos = bit_index(word_index); break; } } - // No bits in range; return end. + } + // For all paths reaching here, (cword != 0) is already known, so we + // expect the compiler to not generate any code for it. Either first word + // was non-zero, or found a non-zero word in range, or fully scanned range + // (so cword is zero). + if (cword != 0) { + idx_t result = cword_pos + count_trailing_zeros(cword); + if (aligned_right || (result < end)) return result; + // Result is beyond range bound; return end. + } + } + return end; +} + +template +inline BitMap::idx_t BitMap::find_last_bit_impl(idx_t beg, idx_t end) const { + STATIC_ASSERT(flip == find_ones_flip || flip == find_zeros_flip); + verify_range(beg, end); + assert(!aligned_left || is_aligned(beg, BitsPerWord), "beg not aligned"); + + if (beg < end) { + // Get the last partial and flipped word in the range. + idx_t last_bit_index = end - 1; + idx_t word_index = to_words_align_down(last_bit_index); + bm_word_t cword = flipped_word(word_index, flip); + // Mask for extracting and testing bits of last word. + bm_word_t last_bit_mask = bm_word_t(1) << bit_in_word(last_bit_index); + if ((cword & last_bit_mask) != 0) { // Test last bit. + return last_bit_index; + } + // Extract prior bits, clearing those above last_bit_index. + cword &= (last_bit_mask - 1); + if (cword == 0) { // Test other bits in the last word. + // Last word had no interesting bits. Word search through + // aligned down beg for a non-zero flipped word. + idx_t word_limit = to_words_align_down(beg); + while (word_index-- > word_limit) { + cword = flipped_word(word_index, flip); + if (cword != 0) break; + } + } + // For all paths reaching here, (cword != 0) is already known, so we + // expect the compiler to not generate any code for it. Either last word + // was non-zero, or found a non-zero word in range, or fully scanned range + // (so cword is zero). + if (cword != 0) { + idx_t result = bit_index(word_index) + log2i(cword); + if (aligned_left || (result >= beg)) return result; + // Result is below range bound; return end. } } return end; @@ -243,6 +293,21 @@ BitMap::find_first_set_bit_aligned_right(idx_t beg, idx_t end) const { return find_first_bit_impl(beg, end); } +inline BitMap::idx_t +BitMap::find_last_set_bit(idx_t beg, idx_t end) const { + return find_last_bit_impl(beg, end); +} + +inline BitMap::idx_t +BitMap::find_last_clear_bit(idx_t beg, idx_t end) const { + return find_last_bit_impl(beg, end); +} + +inline BitMap::idx_t +BitMap::find_last_set_bit_aligned_left(idx_t beg, idx_t end) const { + return find_last_bit_impl(beg, end); +} + // IterateInvoker supports conditionally stopping iteration early. The // invoker is called with the function to apply to each set index, along with // the current index. If the function returns void then the invoker always diff --git a/test/hotspot/gtest/utilities/test_bitMap_search.cpp b/test/hotspot/gtest/utilities/test_bitMap_search.cpp index 7d8c18e946d..84d135ba492 100644 --- a/test/hotspot/gtest/utilities/test_bitMap_search.cpp +++ b/test/hotspot/gtest/utilities/test_bitMap_search.cpp @@ -91,21 +91,34 @@ bool TestIteratorFn::do_bit(size_t offset) { return true; } -static idx_t compute_expected(idx_t search_start, - idx_t search_end, - idx_t left_bit, - idx_t right_bit) { - idx_t expected = search_end; - if (search_start <= left_bit) { - if (left_bit < search_end) { - expected = left_bit; - } - } else if (search_start <= right_bit) { - if (right_bit < search_end) { - expected = right_bit; - } +static bool is_bit_in_range(idx_t bit, idx_t beg, idx_t end) { + return (beg <= bit) && (bit < end); +} + +static idx_t compute_first_expected(idx_t search_start, + idx_t search_end, + idx_t left_bit, + idx_t right_bit) { + if (is_bit_in_range(left_bit, search_start, search_end)) { + return left_bit; + } else if (is_bit_in_range(right_bit, search_start, search_end)) { + return right_bit; + } else { + return search_end; + } +} + +static idx_t compute_last_expected(idx_t search_start, + idx_t search_end, + idx_t left_bit, + idx_t right_bit) { + if (is_bit_in_range(right_bit, search_start, search_end)) { + return right_bit; + } else if (is_bit_in_range(left_bit, search_start, search_end)) { + return left_bit; + } else { + return search_end; } - return expected; } static void test_search_ranges(BitMap& test_ones, @@ -127,6 +140,21 @@ static void test_search_ranges(BitMap& test_ones, EXPECT_EQ(right, test_zeros.find_first_clear_bit(left + 1)); EXPECT_EQ(BITMAP_SIZE, test_zeros.find_first_clear_bit(right + 1)); + // Test find_last_set_bit with full range of map. + EXPECT_EQ(right, test_ones.find_last_set_bit(0)); + EXPECT_EQ(left, test_ones.find_last_set_bit(0, right)); + EXPECT_EQ(left, test_ones.find_last_set_bit(0, left)); + + // Test find_last_set_bit_aligned_left with full range of map. + EXPECT_EQ(right, test_ones.find_last_set_bit_aligned_left(0, BITMAP_SIZE)); + EXPECT_EQ(left, test_ones.find_last_set_bit_aligned_left(0, right)); + EXPECT_EQ(left, test_ones.find_last_set_bit_aligned_left(0, left)); + + // Test find_last_clear_bit with full range of map. + EXPECT_EQ(right, test_zeros.find_last_clear_bit(0)); + EXPECT_EQ(left, test_zeros.find_last_clear_bit(0, right)); + EXPECT_EQ(left, test_zeros.find_last_clear_bit(0, left)); + // Check that iterate invokes the closure function on left and right values. TestIteratorFn test_iteration(0, BITMAP_SIZE, left, right); test_ones.iterate(&test_iteration, 0, BITMAP_SIZE); @@ -165,29 +193,50 @@ static void test_search_ranges(BitMap& test_ones, } bool aligned_right = search_offsets[o_end] == 0; + bool aligned_left = search_offsets[o_start] == 0; ASSERT_LE(start, end); // test bug if fail ASSERT_LT(end, BITMAP_SIZE); // test bug if fail - idx_t expected = compute_expected(start, end, left, right); + idx_t first_expected = compute_first_expected(start, end, left, right); + idx_t last_expected = compute_last_expected(start, end, left, right); - EXPECT_EQ(expected, test_ones.find_first_set_bit(start, end)); - EXPECT_EQ(expected, test_zeros.find_first_clear_bit(start, end)); + EXPECT_EQ(first_expected, test_ones.find_first_set_bit(start, end)); + EXPECT_EQ(first_expected, test_zeros.find_first_clear_bit(start, end)); if (aligned_right) { EXPECT_EQ( - expected, + first_expected, test_ones.find_first_set_bit_aligned_right(start, end)); } - idx_t start2 = MIN2(expected + 1, end); - idx_t expected2 = compute_expected(start2, end, left, right); + EXPECT_EQ(last_expected, test_ones.find_last_set_bit(start, end)); + EXPECT_EQ(last_expected, test_zeros.find_last_clear_bit(start, end)); + if (aligned_left) { + EXPECT_EQ( + last_expected, + test_ones.find_last_set_bit_aligned_left(start, end)); + } - EXPECT_EQ(expected2, test_ones.find_first_set_bit(start2, end)); - EXPECT_EQ(expected2, test_zeros.find_first_clear_bit(start2, end)); + idx_t start2 = MIN2(first_expected + 1, end); + idx_t first_expected2 = compute_first_expected(start2, end, left, right); + + idx_t end2 = MAX2(start, last_expected); + idx_t last_expected2 = compute_last_expected(start, end2, left, right); + + EXPECT_EQ(first_expected2, test_ones.find_first_set_bit(start2, end)); + EXPECT_EQ(first_expected2, test_zeros.find_first_clear_bit(start2, end)); if (aligned_right) { EXPECT_EQ( - expected2, + first_expected2, test_ones.find_first_set_bit_aligned_right(start2, end)); } + + EXPECT_EQ(last_expected2, test_ones.find_last_set_bit(start, end2)); + EXPECT_EQ(last_expected2, test_zeros.find_last_clear_bit(start, end2)); + if (aligned_left) { + EXPECT_EQ( + last_expected2, + test_ones.find_last_set_bit_aligned_left(start, end2)); + } } } }