On 12/30/22 23:33, Alejandro Colomar wrote:
Hi Yann, On 12/30/22 21:33, Alejandro Colomar wrote:On 12/30/22 21:18, Yann Droneaud wrote:What's wrong with the following ?[...]unsigned long max = upper_bound - 1; unsigned long mask = ULONG_MAX >> __builtin_clzl(max);I hate coding these magic operations out of a function, when I can give it a meaningful name. That reads to me as a magic trick that many maintainers that read it after me will blame me for having to parse it.Moreover, it requires you to have the special case for 0 at the top, which I don't want.I reconsidered; my -1 was equally magic. And by calling it 'mask', ULONG_MAX >> n is something not so magic.The builtin still has the problem that it requires special-casing 0, so I prefer the C23 call, which provides the behavior I want for 0:unsigned long shadow_random_uniform(unsigned long upper_bound) { unsigned long r, max, mask; max = upper_bound - 1; mask = ULONG_MAX >> leading_zerosul(max);
I just realized that this isn't good either. Shifting right still has undefined behavior for max = 0. I'll fall back to my original plan.
do { r = shadow_random(); r &= mask; // optimization } while (r > max); return r; }And, now I don't need to add a wrapper around bit_ceil() that removes the UB. stdc_leading_zerosul() is just fine for this use case.Cheers, Alex
-- <http://www.alejandro-colomar.es/>
OpenPGP_signature
Description: OpenPGP digital signature
