repeat: use u32 arithmetic explicitly

In some ring-based models, we know that if the ring is not stale, then
all our bounds should fit within 32-bits. This change makes these
explicitly u32 rather than implicitly narrowing later on.
This commit is contained in:
Justin Viiret 2015-11-03 13:18:34 +11:00 committed by Matthew Barr
parent ae7dbc2472
commit a083bcfa8d
2 changed files with 32 additions and 21 deletions

View File

@ -39,6 +39,8 @@
#include "util/pack_bits.h" #include "util/pack_bits.h"
#include "util/partial_store.h" #include "util/partial_store.h"
#include "util/unaligned.h" #include "util/unaligned.h"
#include <stdint.h>
#include <string.h> #include <string.h>
/** \brief Returns the total capacity of the ring. /** \brief Returns the total capacity of the ring.
@ -709,12 +711,7 @@ enum RepeatMatch repeatHasMatchRing(const struct RepeatInfo *info,
dumpRing(info, xs, ring); dumpRing(info, xs, ring);
#endif #endif
// We work in terms of the distance between the current offset and the base if (offset - xs->offset < info->repeatMin) {
// offset in our history.
u64a delta = offset - xs->offset;
DEBUG_PRINTF("delta=%llu\n", delta);
if (delta < info->repeatMin) {
DEBUG_PRINTF("haven't even seen repeatMin bytes yet!\n"); DEBUG_PRINTF("haven't even seen repeatMin bytes yet!\n");
return REPEAT_NOMATCH; return REPEAT_NOMATCH;
} }
@ -724,17 +721,22 @@ enum RepeatMatch repeatHasMatchRing(const struct RepeatInfo *info,
return REPEAT_STALE; return REPEAT_STALE;
} }
// If we're not stale, delta fits in the range [repeatMin, lastTop +
// repeatMax], which fits in a u32.
assert(offset - xs->offset < UINT32_MAX);
u32 delta = (u32)(offset - xs->offset);
DEBUG_PRINTF("delta=%u\n", delta);
// Find the bounds on possible matches in the ring buffer. // Find the bounds on possible matches in the ring buffer.
u64a lower = delta > info->repeatMax ? delta - info->repeatMax : 0; u32 lower = delta > info->repeatMax ? delta - info->repeatMax : 0;
u64a upper = delta - info->repeatMin + 1; u32 upper = MIN(delta - info->repeatMin + 1, ringOccupancy(xs, ringSize));
upper = MIN(upper, ringOccupancy(xs, ringSize));
if (lower >= upper) { if (lower >= upper) {
DEBUG_PRINTF("no matches to check\n"); DEBUG_PRINTF("no matches to check\n");
return REPEAT_NOMATCH; return REPEAT_NOMATCH;
} }
DEBUG_PRINTF("possible match indices=[%llu,%llu]\n", lower, upper); DEBUG_PRINTF("possible match indices=[%u,%u]\n", lower, upper);
if (ringHasMatch(xs, ring, ringSize, lower, upper)) { if (ringHasMatch(xs, ring, ringSize, lower, upper)) {
return REPEAT_MATCH; return REPEAT_MATCH;
} }
@ -1252,10 +1254,12 @@ u64a repeatNextMatchSparseOptimalP(const struct RepeatInfo *info,
} else if (nextOffset > } else if (nextOffset >
repeatLastTopSparseOptimalP(info, ctrl, state) + repeatLastTopSparseOptimalP(info, ctrl, state) +
info->repeatMax) { info->repeatMax) {
DEBUG_PRINTF("ring is stale\n");
return 0; return 0;
} else { } else {
u64a delta = nextOffset - xs->offset; assert(nextOffset - xs->offset < UINT32_MAX); // ring is not stale
u64a lower = delta > info->repeatMax ? delta - info->repeatMax : 0; u32 delta = (u32)(nextOffset - xs->offset);
u32 lower = delta > info->repeatMax ? delta - info->repeatMax : 0;
patch = lower / patch_size; patch = lower / patch_size;
tval = lower - patch * patch_size; tval = lower - patch * patch_size;
} }
@ -1498,15 +1502,20 @@ enum RepeatMatch repeatHasMatchSparseOptimalP(const struct RepeatInfo *info,
return REPEAT_STALE; return REPEAT_STALE;
} }
u64a delta = offset - xs->offset; // Our delta between the base offset of the ring and the current offset
u64a lower = delta > info->repeatMax ? delta - info->repeatMax : 0; // must fit within the range [repeatMin, lastPossibleTop + repeatMax]. This
u64a upper = delta - info->repeatMin; // range fits comfortably within a u32.
assert(offset - xs->offset <= UINT32_MAX);
u32 delta = (u32)(offset - xs->offset);
u32 patch_size = info->patchSize; u32 patch_size = info->patchSize;
u32 patch_count = info->patchCount; u32 patch_count = info->patchCount;
u32 occ = ringOccupancy(xs, patch_count); u32 occ = ringOccupancy(xs, patch_count);
upper = MIN(upper, occ * patch_size - 1);
DEBUG_PRINTF("lower=%llu, upper=%llu\n", lower, upper); u32 lower = delta > info->repeatMax ? delta - info->repeatMax : 0;
u32 upper = MIN(delta - info->repeatMin, occ * patch_size - 1);
DEBUG_PRINTF("lower=%u, upper=%u\n", lower, upper);
u32 patch_lower = lower / patch_size; u32 patch_lower = lower / patch_size;
u32 patch_upper = upper / patch_size; u32 patch_upper = upper / patch_size;

View File

@ -505,6 +505,7 @@ const RepeatTestInfo sparseRepeats[] = {
{ REPEAT_SPARSE_OPTIMAL_P, 4000, 4000 }, { REPEAT_SPARSE_OPTIMAL_P, 4000, 4000 },
{ REPEAT_SPARSE_OPTIMAL_P, 4500, 4500 }, { REPEAT_SPARSE_OPTIMAL_P, 4500, 4500 },
{ REPEAT_SPARSE_OPTIMAL_P, 5000, 5000 }, { REPEAT_SPARSE_OPTIMAL_P, 5000, 5000 },
{ REPEAT_SPARSE_OPTIMAL_P, 65534, 65534 },
// {N, M} repeats // {N, M} repeats
{ REPEAT_SPARSE_OPTIMAL_P, 10, 20 }, { REPEAT_SPARSE_OPTIMAL_P, 10, 20 },
{ REPEAT_SPARSE_OPTIMAL_P, 20, 40 }, { REPEAT_SPARSE_OPTIMAL_P, 20, 40 },
@ -528,7 +529,8 @@ const RepeatTestInfo sparseRepeats[] = {
{ REPEAT_SPARSE_OPTIMAL_P, 3500, 4000 }, { REPEAT_SPARSE_OPTIMAL_P, 3500, 4000 },
{ REPEAT_SPARSE_OPTIMAL_P, 4000, 8000 }, { REPEAT_SPARSE_OPTIMAL_P, 4000, 8000 },
{ REPEAT_SPARSE_OPTIMAL_P, 4500, 8000 }, { REPEAT_SPARSE_OPTIMAL_P, 4500, 8000 },
{ REPEAT_SPARSE_OPTIMAL_P, 5000, 5001 } { REPEAT_SPARSE_OPTIMAL_P, 5000, 5001 },
{ REPEAT_SPARSE_OPTIMAL_P, 60000, 65534 }
}; };
static static
@ -802,7 +804,7 @@ TEST_P(SparseOptimalTest, Simple1) {
1000 + info->repeatMax * 2)); 1000 + info->repeatMax * 2));
ASSERT_EQ(0U, repeatNextMatch(info, ctrl, state, ASSERT_EQ(0U, repeatNextMatch(info, ctrl, state,
1000 + info->repeatMax * 2 + 1)); 1000 + info->repeatMax * 2 + 1));
ASSERT_EQ(0U, repeatNextMatch(info, ctrl, state, 10000)); ASSERT_EQ(0U, repeatNextMatch(info, ctrl, state, 100000));
} }
TEST_P(SparseOptimalTest, TwoTopsNeg) { TEST_P(SparseOptimalTest, TwoTopsNeg) {