Commit 8ffc8e0a authored by Nick Mathewson's avatar Nick Mathewson

Add comments demonstrating that c[tl]z() input is always nonzero

Given these, we can remove the code to check for the zero case.
parent 7311ebae
...@@ -8,55 +8,46 @@ ...@@ -8,55 +8,46 @@
#if defined(__GNUC__) && !defined(TIMEOUT_DISABLE_GNUC_BITOPS) #if defined(__GNUC__) && !defined(TIMEOUT_DISABLE_GNUC_BITOPS)
/* On GCC and clang and some others, we can use __builtin functions. They /* On GCC and clang and some others, we can use __builtin functions. They
* are not defined for n==0, however.*/ * are not defined for n==0, but timeout.s never calls them with n==0. */
static inline int ctz64(uint64_t n) #define ctz64(n) __builtin_ctzll(n)
{ #define clz64(n) __builtin_clzll(n)
return n ? __builtin_ctzll(n) : 64;
}
static inline int clz64(uint64_t n)
{
return n ? __builtin_clzll(n) : 64;
}
#if LONG_BITS == 32 #if LONG_BITS == 32
#define ctz32_(n) __builtin_ctzl(n) #define ctz32(n) __builtin_ctzl(n)
#define clz32_(n) __builtin_clzl(n) #define clz32(n) __builtin_clzl(n)
#else #else
#define ctz32_(n) __builtin_ctz(n) #define ctz32(n) __builtin_ctz(n)
#define clz32_(n) __builtin_clz(n) #define clz32(n) __builtin_clz(n)
#endif #endif
static inline int ctz32(uint64_t n)
{
return n ? ctz32_(n) : 32;
}
static inline int clz32(uint64_t n)
{
return n ? clz32_(n) : 32;
}
#elif defined(_MSC_VER) && !defined(TIMEOUT_DISABLE_MSVC_BITOPS) #elif defined(_MSC_VER) && !defined(TIMEOUT_DISABLE_MSVC_BITOPS)
/* On MSVC, we have these handy functions */ /* On MSVC, we have these handy functions. We can ignore their return
* values, since we will never supply val == 0. */
static __inline int ctz64(uint64_t val) static __inline int ctz64(uint64_t val)
{ {
DWORD zeros = 0; DWORD zeros = 0;
return _BitScanForward64(&zeros, val) ? zeros : 64; _BitScanForward64(&zeros, val);
return zeros;
} }
static __inline int clz64(uint64_t val) static __inline int clz64(uint64_t val)
{ {
DWORD zeros = 0; DWORD zeros = 0;
return _BitScanReverse64(&zeros, val) ? zeros : 64; _BitScanReverse64(&zeros, val);
return zeros;
} }
static __inline int ctz32(unsigned long val) static __inline int ctz32(unsigned long val)
{ {
DWORD zeros = 0; DWORD zeros = 0;
return _BitScanForward(&zeros, val) ? zeros : 32; _BitScanForward(&zeros, val);
return zeros;
} }
static __inline int clz32(unsigned long val) static __inline int clz32(unsigned long val)
{ {
DWORD zeros = 0; DWORD zeros = 0;
return _BitScanReverse(&zeros, val) ? zeros : 32; _BitScanReverse(&zeros, val);
return zeros;
} }
/* End of MSVC case. */ /* End of MSVC case. */
...@@ -109,8 +100,6 @@ static inline int clz32(uint32_t x) ...@@ -109,8 +100,6 @@ static inline int clz32(uint32_t x)
static inline int ctz64(uint64_t x) static inline int ctz64(uint64_t x)
{ {
int rv = 0; int rv = 0;
if (!x)
return 64;
process64(32); process64(32);
process64(16); process64(16);
...@@ -125,8 +114,6 @@ static inline int ctz64(uint64_t x) ...@@ -125,8 +114,6 @@ static inline int ctz64(uint64_t x)
static inline int ctz32(uint32_t x) static inline int ctz32(uint32_t x)
{ {
int rv = 0; int rv = 0;
if (!x)
return 32;
process32(16); process32(16);
process32(8); process32(8);
...@@ -189,6 +176,9 @@ check(uint64_t vv) ...@@ -189,6 +176,9 @@ check(uint64_t vv)
{ {
uint32_t v32 = (uint32_t) vv; uint32_t v32 = (uint32_t) vv;
if (vv == 0)
return 1; /* c[tl]z64(0) is undefined. */
if (ctz64(vv) != naive_ctz(64, vv)) { if (ctz64(vv) != naive_ctz(64, vv)) {
printf("mismatch with ctz64: %d\n", ctz64(vv)); printf("mismatch with ctz64: %d\n", ctz64(vv));
exit(1); exit(1);
...@@ -199,6 +189,10 @@ check(uint64_t vv) ...@@ -199,6 +189,10 @@ check(uint64_t vv)
exit(1); exit(1);
return 0; return 0;
} }
if (v32 == 0)
return 1; /* c[lt]z(0) is undefined. */
if (ctz32(v32) != naive_ctz(32, v32)) { if (ctz32(v32) != naive_ctz(32, v32)) {
printf("mismatch with ctz32: %d\n", ctz32(v32)); printf("mismatch with ctz32: %d\n", ctz32(v32));
exit(1); exit(1);
......
...@@ -299,6 +299,7 @@ static inline reltime_t timeout_rem(struct timeouts *T, struct timeout *to) { ...@@ -299,6 +299,7 @@ static inline reltime_t timeout_rem(struct timeouts *T, struct timeout *to) {
static inline int timeout_wheel(timeout_t timeout) { static inline int timeout_wheel(timeout_t timeout) {
/* must be called with timeout != 0, so fls input is nonzero */
return (fls(MIN(timeout, TIMEOUT_MAX)) - 1) / WHEEL_BIT; return (fls(MIN(timeout, TIMEOUT_MAX)) - 1) / WHEEL_BIT;
} /* timeout_wheel() */ } /* timeout_wheel() */
...@@ -321,6 +322,11 @@ static void timeouts_sched(struct timeouts *T, struct timeout *to, timeout_t exp ...@@ -321,6 +322,11 @@ static void timeouts_sched(struct timeouts *T, struct timeout *to, timeout_t exp
if (expires > T->curtime) { if (expires > T->curtime) {
rem = timeout_rem(T, to); rem = timeout_rem(T, to);
/* rem is nonzero since:
* rem == timeout_rem(T,to),
* == to->expires - T->curtime
* and above we have expires > T->curtime.
*/
wheel = timeout_wheel(rem); wheel = timeout_wheel(rem);
slot = timeout_slot(wheel, to->expires); slot = timeout_slot(wheel, to->expires);
...@@ -416,6 +422,7 @@ TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) { ...@@ -416,6 +422,7 @@ TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) {
} }
while (pending & T->pending[wheel]) { while (pending & T->pending[wheel]) {
/* ctz input cannot be zero: loop condition. */
int slot = ctz(pending & T->pending[wheel]); int slot = ctz(pending & T->pending[wheel]);
TAILQ_CONCAT(&todo, &T->wheel[wheel][slot], tqe); TAILQ_CONCAT(&todo, &T->wheel[wheel][slot], tqe);
T->pending[wheel] &= ~(UINT64_C(1) << slot); T->pending[wheel] &= ~(UINT64_C(1) << slot);
...@@ -492,6 +499,8 @@ static timeout_t timeouts_int(struct timeouts *T) { ...@@ -492,6 +499,8 @@ static timeout_t timeouts_int(struct timeouts *T) {
if (T->pending[wheel]) { if (T->pending[wheel]) {
slot = WHEEL_MASK & (T->curtime >> (wheel * WHEEL_BIT)); slot = WHEEL_MASK & (T->curtime >> (wheel * WHEEL_BIT));
/* ctz input cannot be zero: T->pending[wheel] is
* nonzero, so rotr() is nonzero. */
_timeout = (ctz(rotr(T->pending[wheel], slot)) + !!wheel) << (wheel * WHEEL_BIT); _timeout = (ctz(rotr(T->pending[wheel], slot)) + !!wheel) << (wheel * WHEEL_BIT);
/* +1 to higher order wheels as those timeouts are one rotation in the future (otherwise they'd be on a lower wheel or expired) */ /* +1 to higher order wheels as those timeouts are one rotation in the future (otherwise they'd be on a lower wheel or expired) */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment