diff --git a/thiersort.h b/thiersort.h index f5c61fa..f7207ff 100644 --- a/thiersort.h +++ b/thiersort.h @@ -421,17 +421,17 @@ static inline void thiersort_apply( TSU32 elemsize, void *tmp, void (*free)(void *ptr)) { - // Replace all positions + /* Replace all positions */ for(TSU32 i = 0; i < length; ++i) { - // Replace the whole chain starting at i (j for chain index) - // This works because we can easily check what does'nt need moves anymore. + /* Replace the whole chain starting at i (j for chain index) */ + /* This works because we can easily check what does'nt need moves anymore. */ TSU32 j = i; TSU32 ni = sortres[j].i; - // Until would move to its own position, chain - // This also solves "already at right place" originals. + /* Until would move to its own position, chain */ + /* This also solves "already at right place" originals. */ while(ni != j) { - // xchg j and ni + /* xchg j and ni */ memcpy(tmp, &((TSU8*)arr)[(size_t)j * elemsize], elemsize); memcpy( &((TSU8*)arr)[(size_t)j * elemsize], @@ -439,14 +439,14 @@ static inline void thiersort_apply( elemsize); memcpy(&((TSU8*)arr)[(size_t)ni * elemsize], tmp, elemsize); - // Mark j index as done in sortres for outer loop. - // This is necessary for inner loop stopping early - // and outer catch up on already processed location. + /* Mark j index as done in sortres for outer loop. */ + /* This is necessary for inner loop stopping early */ + /* and outer catch up on already processed location. */ sortres[j].i = j; - // Update j and ni - // Now we must find what should be at new location j - // instead of what we swap in there from old j loc. + /* Update j and ni */ + /* Now we must find what should be at new location j */ + /* instead of what we swap in there from old j loc. */ j = ni; ni = sortres[j].i; } @@ -518,7 +518,8 @@ static inline TSU8 ts_radixi( return ts_floatsigntrick((TSU8)k.u); } - assert(false); // should never happen + /* Should never happen */ + assert(false); } /** Simple inplace quicksort for tselems */ @@ -533,8 +534,10 @@ static inline void ts_quicksort_inplace( const TSU32 bi, void *reent_data), void *reent_data) { - // Must do this early exit! - if(from >= to) return; + /* Must do this early exit! */ + // TODO: TS_UNLIKELY + if(to == 0) return; + if(from >= to - 1) return; TSU32 len = (to - from); TSU32 mid = from + len / 2; @@ -544,7 +547,7 @@ static inline void ts_quicksort_inplace( TSU32 left = from; TSU32 right = to - 1; while(left < right) { - // Step over rightly positioned elems from left + /* Step over rightly positioned elems from left */ union tskey leftkey = arr[left].key; TSU32 lefti = arr[left].i; while((left < right) && lt(leftkey, pivotkey, lefti, pivoti, reent_data)) { @@ -553,16 +556,16 @@ static inline void ts_quicksort_inplace( lefti = arr[left].i; } - // Step over rightly positioned elems from right + /* Step over rightly positioned elems from right */ union tskey rightkey = arr[right].key; TSU32 righti = arr[right].i; - while((left < right) && lt(pivotkey, rightkey, pivoti, righti, reent_data)) { + while((left < right) && !lt(rightkey, pivotkey, righti, pivoti, reent_data)) { --right; rightkey = arr[right].key; righti = arr[right].i; } - // Swap wrongly positioned element + /* Swap wrongly positioned element */ if(left < right) { const struct tselem tmp = arr[left]; arr[left] = arr[right]; @@ -579,9 +582,31 @@ static inline void ts_quicksort_inplace( left += (lt(leftkey, pivotkey, lefti, pivoti, reent_data)); } - /* Just simple recursion for now */ - ts_quicksort_inplace(arr, from, left, lt, reent_data); - ts_quicksort_inplace(arr, left, to, lt, reent_data); + /* Check edge-case when left got empty. */ + /* Rem.: Right moves over the pivot invariably as left cannot pass over if! */ + /* This means that right subset can never be empty, but left can! */ + /* This also means that if left is empty, all invariants were just */ + /* stepped over (from right) and not moved from its original position! */ + // TODO: TS_UNLIKELY + if(from < left) { + /** Good case: Somewhere cutting the array in around half */ + ts_quicksort_inplace(arr, from, left, lt, reent_data); + ts_quicksort_inplace(arr, left, to, lt, reent_data); + } else { + /* Left sub-array got to be empty - but to step with recursion */ + /* we need to shrink sizes, which is possible by moving the pivot */ + /* to the start of the right (because we know its a minimal elem!) */ + /* XXX: If we collect up all pivots, we can move all here if want.. */ + + /* Swap */ + const struct tselem tmp = arr[left]; + arr[left] = arr[mid]; + arr[mid] = tmp; + + /* Bad case: could shrink the array by one element only */ + ts_quicksort_inplace(arr, left + 1, to, lt, reent_data); + } + } /** @@ -601,7 +626,7 @@ static inline void ts_quicksort_fromto( const TSU32 bi, void *reent_data), void *reent_data) { - // Must do this early exit! + // FIXME: Code is rotten here - see above variant pls! if(from >= to) return; TSU32 len = (to - from); @@ -650,7 +675,7 @@ static inline void thiersort8_internal( /* Step 1) Bucketing */ #ifdef TS_SSE2 - // TODO: implement with SSE2 + /* TODO: implement with SSE2 */ #else /* TODO: We can go both down and upwards here to increase ILP! */ /* Occurence counting O(n) */