On Thu, 27 Nov 2025, Alexandre Oliva wrote:
> On Nov 26, 2025, Vladimir Makarov <[email protected]> wrote:
>
> > I any case any saving is welcome. Taking also into account that the
> > iterator is simpler, I'd go for this change.
>
> > So, Alex, if you want you can commit the patch (of course after usual
> > testing).
>
> I'd already tested it, but I've added some comments, so I gave it
> another round of testing, and a ChangeLog entry.
>
> Here's what I'm going to put in, barring objections. Regstrapped on
> x86_64-linux-gnu.
>
>
> Use the newly-added first_diff API for iteration. This hugely
> simplifies the iterator type, that no longer saves internal state, and
> largely simplifies and likely speeds up iteration, thanks to the use
> of ctz.
>
>
> for gcc/ChangeLog
>
> PR rtl-optimization/122767
> * hard-reg-set.h (hard_reg_set_iterator): Simplify.
> (hard_reg_set_iter_init): Likewise.
> (hard_reg_set_iter_set): Likewise. Use first_diff.
> (hard_reg_set_iter_next): Likewise.
> ---
> gcc/hard-reg-set.h | 77
> ++++++++++------------------------------------------
> 1 file changed, 15 insertions(+), 62 deletions(-)
>
> diff --git a/gcc/hard-reg-set.h b/gcc/hard-reg-set.h
> index e33c8e334d396..92f5531cb2016 100644
> --- a/gcc/hard-reg-set.h
> +++ b/gcc/hard-reg-set.h
> @@ -321,23 +321,13 @@ hard_reg_set_first_diff (const_hard_reg_set x,
> const_hard_reg_set y,
> }
> #endif
>
> -/* Iterator for hard register sets. */
> +/* Iterator for hard register sets. This is not an iterator proper, the bulk
> + of the iterator state is held in the REGNO passed to the iterator
> + functions. */
>
> struct hard_reg_set_iterator
> {
> - /* Pointer to the current element. */
> - const HARD_REG_ELT_TYPE *pelt;
> -
> - /* The length of the set. */
> - unsigned short length;
> -
> - /* Word within the current element. */
> - unsigned short word_no;
> -
> - /* Contents of the actually processed word. When finding next bit
> - it is shifted right, so that the actual bit is always the least
> - significant bit of ACTUAL. */
> - HARD_REG_ELT_TYPE bits;
> + const HARD_REG_SET *set;
> };
>
> #define HARD_REG_ELT_BITS UHOST_BITS_PER_WIDE_INT
> @@ -348,64 +338,27 @@ inline void
> hard_reg_set_iter_init (hard_reg_set_iterator *iter, const_hard_reg_set set,
> unsigned min, unsigned *regno)
> {
> -#ifdef HARD_REG_SET_LONGS
> - iter->pelt = set.elts;
> - iter->length = HARD_REG_SET_LONGS;
> -#else
> - iter->pelt = &set;
> - iter->length = 1;
> -#endif
> - iter->word_no = min / HARD_REG_ELT_BITS;
> - if (iter->word_no < iter->length)
> - {
> - iter->bits = iter->pelt[iter->word_no];
> - iter->bits >>= min % HARD_REG_ELT_BITS;
> -
> - /* This is required for correct search of the next bit. */
> - min += !iter->bits;
> - }
> + iter->set = &set;
> *regno = min;
> }
>
> inline bool
> hard_reg_set_iter_set (hard_reg_set_iterator *iter, unsigned *regno)
> {
> - while (1)
> - {
> - /* Return false when we're advanced past the end of the set. */
> - if (iter->word_no >= iter->length)
> - return false;
> -
> - if (iter->bits)
> - {
> - /* Find the correct bit and return it. */
> - while (!(iter->bits & 1))
> - {
> - iter->bits >>= 1;
> - *regno += 1;
> - }
> - return (*regno < FIRST_PSEUDO_REGISTER);
> - }
> -
> - /* Round to the beginning of the next word. */
> - *regno = (*regno + HARD_REG_ELT_BITS - 1);
> - *regno -= *regno % HARD_REG_ELT_BITS;
> -
> - /* Find the next non-zero word. */
> - while (++iter->word_no < iter->length)
> - {
> - iter->bits = iter->pelt[iter->word_no];
> - if (iter->bits)
> - break;
> - *regno += HARD_REG_ELT_BITS;
> - }
> - }
> + static const HARD_REG_SET hard_reg_set_empty = {};
> + *regno = std::abs (hard_reg_set_first_diff (*iter->set,
> + hard_reg_set_empty,
> + *regno)) - 1;
I'm not sure I like using hard_reg_set_first_diff, esp. when
that ends up using the array version this looks like a step
backwards?
Richard.
> + /* If hard_reg_set_first_diff returns zero, we'll have set *regno to
> + (unsigned)-1 above, so the test below will yield FALSE, and iteration
> will
> + stop. Also stop iteration if it finds a nonzero bit at
> + FIRST_PSEUDO_REGISTER or greater. */
> + return *regno < FIRST_PSEUDO_REGISTER;
> }
>
> inline void
> -hard_reg_set_iter_next (hard_reg_set_iterator *iter, unsigned *regno)
> +hard_reg_set_iter_next (hard_reg_set_iterator *, unsigned *regno)
> {
> - iter->bits >>= 1;
> *regno += 1;
> }
>
>
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
