Konstantinos Eleftheriou <konstantinos.elefther...@vrull.eu> writes:
> Hi Richard, thanks for your response.
>
> On Tue, May 20, 2025 at 8:05 AM Richard Biener
> <richard.guent...@gmail.com> wrote:
>>
>> On Mon, May 19, 2025 at 4:14 PM Konstantinos Eleftheriou
>> <konstantinos.elefther...@vrull.eu> wrote:
>> >
>> > This patch adds the `bitmap_is_range_set_p` function in sbitmap,
>> > which checks if all the bits in a range are set. This function
>> > calls `bitmap_bit_in_range_p_1`, which has been updated to use
>> > the `any_inverted` parameter. When `any_inverted` is true, the helper
>> > function checks if any of the bits in the range is unset, instead of
>> > checking the opposite.
>> >
>> > Function `bitmap_bit_in_range_p` has been updated to call
>> > `bitmap_bit_in_range_p_1` with the `any_inverted` parameter
>> > set to false, retaining its previous functionality.
>> >
>> > Function `bitmap_is_range_set_p` calls `bitmap_bit_in_range_p_1`
>> > with `any_inverted` set to true and returns the negation of the
>> > result, i.e. true if all the bits in the range are set.
>> >
>> > gcc/ChangeLog:
>> >
>> > * sbitmap.cc (bitmap_bit_in_range_p_1): Added the `any_inverted`
>> > parameter and changed the logic to check if any of the bits in
>> > the range is unset, when the value of the parameter is "true".
>> > (bitmap_is_range_set_p): New function.
>> > (bitmap_bit_in_range_p): Call and return the result of
>> > `bitmap_bit_in_range_p_1` with the `any_inverted` parameter set
>> > to false.
>> > * sbitmap.h (bitmap_is_range_set_p): New function.
>> >
>> > Signed-off-by: Konstantinos Eleftheriou <konstantinos.elefther...@vrull.eu>
>> > ---
>> >
>> > (no changes since v1)
>> >
>> > gcc/sbitmap.cc | 27 ++++++++++++++++++++-------
>> > gcc/sbitmap.h | 1 +
>> > 2 files changed, 21 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/gcc/sbitmap.cc b/gcc/sbitmap.cc
>> > index 94f2bbd6c8fd..99f1db540ab6 100644
>> > --- a/gcc/sbitmap.cc
>> > +++ b/gcc/sbitmap.cc
>> > @@ -326,12 +326,13 @@ bitmap_set_range (sbitmap bmap, unsigned int start,
>> > unsigned int count)
>> > bmap->elms[start_word] |= mask;
>> > }
>> >
>> > -/* Return TRUE if any bit between START and END inclusive is set within
>> > - the simple bitmap BMAP. Return FALSE otherwise. */
>> > +/* Helper function for bitmap_bit_in_range_p and bitmap_is_range_set_p.
>> > + If ANY_INVERTED is true, the function checks if any bit in the range
>> > + is unset. */
>> >
>> > bool
>> > bitmap_bit_in_range_p_1 (const_sbitmap bmap, unsigned int start,
>> > - unsigned int end)
>> > + unsigned int end, bool any_inverted)
>> > {
>> > gcc_checking_assert (start <= end);
>> > bitmap_check_index (bmap, end);
>> > @@ -351,7 +352,8 @@ bitmap_bit_in_range_p_1 (const_sbitmap bmap, unsigned
>> > int start,
>> >
>> > SBITMAP_ELT_TYPE low_mask = ((SBITMAP_ELT_TYPE)1 << start_bitno) -
>> > 1;
>> > SBITMAP_ELT_TYPE mask = high_mask - low_mask;
>> > - if (bmap->elms[start_word] & mask)
>> > + const SBITMAP_ELT_TYPE expected_partial = any_inverted ? mask : 0;
>> > + if ((bmap->elms[start_word] & mask) != expected_partial)
>> > return true;
>> > start_word++;
>> > }
>> > @@ -361,9 +363,10 @@ bitmap_bit_in_range_p_1 (const_sbitmap bmap, unsigned
>> > int start,
>> >
>> > /* Now test words at a time until we hit a partial word. */
>> > unsigned int nwords = (end_word - start_word);
>> > + const SBITMAP_ELT_TYPE expected = any_inverted ? ~(SBITMAP_ELT_TYPE)0 :
>> > 0;
>> > while (nwords)
>> > {
>> > - if (bmap->elms[start_word])
>> > + if (bmap->elms[start_word] != expected)
>> > return true;
>> > start_word++;
>> > nwords--;
>> > @@ -373,7 +376,17 @@ bitmap_bit_in_range_p_1 (const_sbitmap bmap, unsigned
>> > int start,
>> > SBITMAP_ELT_TYPE mask = ~(SBITMAP_ELT_TYPE)0;
>> > if (end_bitno + 1 < SBITMAP_ELT_BITS)
>> > mask = ((SBITMAP_ELT_TYPE)1 << (end_bitno + 1)) - 1;
>> > - return (bmap->elms[start_word] & mask) != 0;
>> > + const SBITMAP_ELT_TYPE expected_partial = any_inverted ? mask : 0;
>> > + return (bmap->elms[start_word] & mask) != expected_partial;
>> > +}
>> > +
>> > +/* Return TRUE if all bits between START and END inclusive are set within
>> > + the simple bitmap BMAP. Return FALSE otherwise. */
>> > +
>> > +bool
>> > +bitmap_is_range_set_p (const_sbitmap bmap, unsigned int start, unsigned
>> > int end)
>> > +{
>> > + return !bitmap_bit_in_range_p_1 (bmap, start, end, true);
>> > }
>> >
>> > /* Return TRUE if any bit between START and END inclusive is set within
>> > @@ -382,7 +395,7 @@ bitmap_bit_in_range_p_1 (const_sbitmap bmap, unsigned
>> > int start,
>> > bool
>> > bitmap_bit_in_range_p (const_sbitmap bmap, unsigned int start, unsigned
>> > int end)
>> > {
>> > - return bitmap_bit_in_range_p_1 (bmap, start, end);
>> > + return bitmap_bit_in_range_p_1 (bmap, start, end, false);
>> > }
>> >
>> > #if GCC_VERSION < 3400
>> > diff --git a/gcc/sbitmap.h b/gcc/sbitmap.h
>> > index 66f9e138503c..4ff93e7a98f9 100644
>> > --- a/gcc/sbitmap.h
>> > +++ b/gcc/sbitmap.h
>> > @@ -288,6 +288,7 @@ extern bool bitmap_ior (sbitmap, const_sbitmap,
>> > const_sbitmap);
>> > extern bool bitmap_xor (sbitmap, const_sbitmap, const_sbitmap);
>> > extern bool bitmap_subset_p (const_sbitmap, const_sbitmap);
>> > extern bool bitmap_bit_in_range_p (const_sbitmap, unsigned int, unsigned
>> > int);
>> > +extern bool bitmap_is_range_set_p (const_sbitmap, unsigned int, unsigned
>> > int);
>>
>> To me those sound like doing the same thing. Maybe all_bits_in_range_p vs.
>> any_bit_in_range_p?
Yeah, that sounds less ambiguous to me too FWIW.
> Do we need to rename bit_in_range_p to any_bit_in_range_p (this would
> be renamed globally)?
It would, but it looks like the only non-testing uses are in asf and
tree-ssa-dse.cc, so a renaming shouldn't be too bad (I hope).
Given that I just sent a message saying that the current patch 1 could
be folded into this one, I suppose I should say that a patch to rename
the existing function would be better as a separate patch in the series.
Thanks,
Richard
