On Mon, 15 Jun 2026, Tamar Christina wrote:
> > > --- a/gcc/config/aarch64/aarch64-early-ra.cc > > > +++ b/gcc/config/aarch64/aarch64-early-ra.cc > > > @@ -3038,7 +3038,14 @@ early_ra::allocate_colors () > > > if (dump_file && (dump_flags & TDF_DETAILS)) > > > fprintf (dump_file, " Allocating [v%d:v%d] to color %d\n", > > > best, best + color->group->size - 1, color->id); > > > - m_allocated_fprs |= ((1U << color->group->size) - 1) << best; > > > + // Mark the COLOR's FPRs as allocated. Build the mask with a loop > > > + // rather than ((1U << size) - 1), since a full-width color can > > > have > > > + // size == 32 and "1U << 32" would be undefined. The candidate > > > + // search above guarantees best + size <= 32, so each shift is in > > > + // range. > > > + gcc_assert (best + color->group->size <= 32); > > > + for (unsigned int i = 0; i < color->group->size; ++i) > > > + m_allocated_fprs |= 1U << (best + i); > > > > Why not just: > > gcc_assert (best + color->group->size <= 32); > > if (best + color->group->size == 32) > > m_allocated_fprs = -1U; > > else > > m_allocated_fprs |= ((1U << color->group->size) - 1) << best; > > > > The loop seems a bit much here. I would agree. > Ikn, I prefer the loop because it's at least clear to what it's doing and > doesn't rely > on the size of m_allocated_fprs being the size of an unsigned int. I'm not sure how, it still relies on 1u (and not 1ull) being sufficiently wide in '1U << (best + i)'. > otherwise > > m_allocated_fprs |= ((1ULL << color->group->size) - 1) << best; > > would work too and avoid the branch entirely. Surely we can find a nicer way to express this, for instance: ~(~0u << color->group->size) << best Alexander
