On Mon, 15 Jun 2026, Kyrylo Tkachov wrote:

> >> 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
> 
> FWIW I think Tamar’s version is a bit more readable, though at this point this
> is a subjective cosmetic choice.  Given I’ve tested Tamar’s version and this
> is a serious-ish wrong-code bug I’ll go ahead and commit that.

Thank you for the quick investigation and fix, that was impressive!

I think I should add that I momentarily lost sight of the initial problem when
writing my suggestion above, and it still fails when color->group->size == 32.
>From context it looks like color->group->size cannot be zero, and if so, it
should still be possible to write it without resorting to 64-bit intermediates:

  0xFFFF'FFFF >> (32 - color->group->size) << best

(just to illustrate that in a wider 64-bit context one wouldn't need a 128-bit
intermediate)

Thanks again.
Alexander

Reply via email to