Skip to content

Commit

Permalink
Manually fix several for loops
Browse files Browse the repository at this point in the history
I used this replacement, and then went through each and reasoned if wrapping can ever happen (mostly it cannot, e.g. -1 is ok if for loop starts at 1)

When reviewing, make sure to use "ignore blanks" because lots of code was only de-indented.

```diff
@@
expression idx, start, end;
@@
-   idx = start;
-   while idx < end
+   for idx in start..end
    {
        {
        ...
        }
-        idx = idx.wrapping_add(1);
    }

@@
expression idx, start, end;
@@
-   idx = start;
-   while idx <= end
+   for idx in start..=end
    {
        {
        ...
        }
-        idx = idx.wrapping_add(1);
    }
```
  • Loading branch information
nyurik authored and danielrh committed Apr 6, 2024
1 parent d7481f6 commit 462f73c
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 297 deletions.
116 changes: 46 additions & 70 deletions src/enc/backward_references/hq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl<AllocF: Allocator<floatX>> ZopfliCostModel<AllocF> {
} else {
AllocF::AllocatedMemory::default()
},
distance_histogram_size: min(dist.alphabet_size, 544),
distance_histogram_size: min(dist.alphabet_size, 544),
}
}

Expand Down Expand Up @@ -401,12 +401,8 @@ where
);
matches_offset += loc_offset;
}
i = 0usize;
while i <= 37usize {
{
dict_matches[i] = kInvalidMatch;
}
i = i.wrapping_add(1);
for i in 0..=37 {
dict_matches[i] = kInvalidMatch
}
{
let minlen = max(4, best_len.wrapping_add(1));
Expand All @@ -421,27 +417,22 @@ where
{
assert!(params.use_dictionary);
let maxlen = min(37, max_length);
let mut l: usize;
l = minlen;
while l <= maxlen {
{
let dict_id: u32 = dict_matches[l];
if dict_id < kInvalidMatch {
let distance: usize = max_backward
.wrapping_add(gap)
.wrapping_add((dict_id >> 5) as usize)
.wrapping_add(1);
if distance <= params.dist.max_distance {
BackwardMatchMut(&mut matches[matches_offset]).init_dictionary(
distance,
l,
(dict_id & 31u32) as usize,
);
matches_offset += 1;
}
for l in minlen..=maxlen {
let dict_id: u32 = dict_matches[l];
if dict_id < kInvalidMatch {
let distance: usize = max_backward
.wrapping_add(gap)
.wrapping_add((dict_id >> 5) as usize)
.wrapping_add(1);
if distance <= params.dist.max_distance {
BackwardMatchMut(&mut matches[matches_offset]).init_dictionary(
distance,
l,
(dict_id & 31u32) as usize,
);
matches_offset += 1;
}
}
l = l.wrapping_add(1);
}
}
}
Expand Down Expand Up @@ -537,20 +528,13 @@ fn StartPosQueuePush(xself: &mut StartPosQueue, posdata: &PosData) {
let mut offset: usize = !xself.idx_ & 7usize;
xself.idx_ = xself.idx_.wrapping_add(1);
let len: usize = StartPosQueueSize(xself);
let mut i: usize;
let q: &mut [PosData; 8] = &mut xself.q_;
q[offset] = *posdata;
i = 1usize;
while i < len {
{
if (q[(offset & 7usize)]).costdiff > (q[(offset.wrapping_add(1) & 7usize)]).costdiff {
let mut __brotli_swap_tmp: PosData = q[(offset & 7usize)];
q[(offset & 7usize)] = q[(offset.wrapping_add(1) & 7usize)];
q[(offset.wrapping_add(1) & 7usize)] = __brotli_swap_tmp;
}
offset = offset.wrapping_add(1);
for _i in 1..len {
if q[offset & 7].costdiff > q[(offset + 1) & 7].costdiff {
q.swap(offset & 7, (offset + 1) & 7);
}
i = i.wrapping_add(1);
offset = offset.wrapping_add(1);
}
}

Expand Down Expand Up @@ -780,41 +764,33 @@ fn UpdateNodes<AllocF: Allocator<floatX>>(
}
{
let dist_cost = base_cost + model.get_distance_cost(j);
let mut l: usize;
l = best_len.wrapping_add(1);
while l <= len {
for l in best_len.wrapping_add(1)..=len {
let copycode: u16 = GetCopyLengthCode(l);
let cmdcode: u16 =
CombineLengthCodes(inscode, copycode, (j == 0usize) as i32);
let cost: floatX =
(if cmdcode < 128 { base_cost } else { dist_cost })
+ (GetCopyExtra(copycode) as floatX)
+ model.get_command_cost(cmdcode);
if cost
< match (nodes[pos.wrapping_add(l)]).u {
Union1::cost(cost) => cost,
_ => 0.0,
}
{
let copycode: u16 = GetCopyLengthCode(l);
let cmdcode: u16 = CombineLengthCodes(
inscode,
copycode,
(j == 0usize) as i32,
UpdateZopfliNode(
nodes,
pos,
start,
l,
l,
backward,
j.wrapping_add(1),
cost,
);
let cost: floatX =
(if cmdcode < 128 { base_cost } else { dist_cost })
+ (GetCopyExtra(copycode) as floatX)
+ model.get_command_cost(cmdcode);
if cost
< match (nodes[pos.wrapping_add(l)]).u {
Union1::cost(cost) => cost,
_ => 0.0,
}
{
UpdateZopfliNode(
nodes,
pos,
start,
l,
l,
backward,
j.wrapping_add(1),
cost,
);
result = max(result, l);
}
best_len = l;
result = max(result, l);
}
l = l.wrapping_add(1);
best_len = l;
}
}
}
Expand Down Expand Up @@ -1212,7 +1188,7 @@ impl<AllocF: Allocator<floatX>> ZopfliCostModel<AllocF> {
self.cost_dist_.slice_mut(),
);
for i in 0usize..704usize {
min_cost_cmd = min_cost_cmd.min(cost_cmd[i]);
min_cost_cmd = min_cost_cmd.min(cost_cmd[i]);
}
self.min_cost_cmd_ = min_cost_cmd;
{
Expand Down
13 changes: 4 additions & 9 deletions src/enc/bit_cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,16 +294,11 @@ pub fn BrotliPopulationCost<HistogramType: SliceWrapper<u32> + CostAccessors>(
for i in 0usize..4usize {
histo[i] = histogram.slice()[s[i]];
}
for i in 0usize..4usize {
let mut j: usize;
j = i.wrapping_add(1);
while j < 4usize {
{
if histo[j] > histo[i] {
histo.swap(j, i);
}
for i in 0..4 {
for j in i + 1..4 {
if histo[j] > histo[i] {
histo.swap(j, i);
}
j = j.wrapping_add(1);
}
}
let h23: u32 = histo[2].wrapping_add(histo[3]);
Expand Down
90 changes: 26 additions & 64 deletions src/enc/brotli_bit_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -846,21 +846,15 @@ fn BrotliStoreHuffmanTreeOfHuffmanTreeToBitMask(
}
}
BrotliWriteBits(2, skip_some, storage_ix, storage);
{
let mut i: u64;
i = skip_some;
while i < codes_to_store {
{
let l: usize = code_length_bitdepth[(kStorageOrder[i as usize] as usize)] as usize;
BrotliWriteBits(
kHuffmanBitLengthHuffmanCodeBitLengths[l],
kHuffmanBitLengthHuffmanCodeSymbols[l] as u64,
storage_ix,
storage,
);
}
i = i.wrapping_add(1);
}

for i in skip_some..codes_to_store {
let l = code_length_bitdepth[kStorageOrder[i as usize] as usize] as usize;
BrotliWriteBits(
kHuffmanBitLengthHuffmanCodeBitLengths[l],
kHuffmanBitLengthHuffmanCodeSymbols[l] as u64,
storage_ix,
storage,
);
}
}

Expand Down Expand Up @@ -1107,26 +1101,14 @@ pub fn BrotliBuildAndStoreHuffmanTreeFast<AllocHT: alloc::Allocator<HuffmanTree>
}
BrotliConvertBitDepthsToSymbols(depth, length as usize, bits);
if count <= 4 {
let mut i: u64;
BrotliWriteBits(2, 1, storage_ix, storage);
BrotliWriteBits(2, count.wrapping_sub(1), storage_ix, storage);
i = 0;
while i < count {
{
let mut j: u64;
j = i.wrapping_add(1);
while j < count {
{
if (depth[(symbols[j as usize] as usize)] as i32)
< depth[(symbols[i as usize] as usize)] as i32
{
symbols.swap(j as usize, i as usize);
}
}
j = j.wrapping_add(1);
for i in 0..count as usize {
for j in i + 1..count as usize {
if depth[symbols[j] as usize] < depth[symbols[i] as usize] {
symbols.swap(j, i);
}
}
i = i.wrapping_add(1);
}
if count == 2 {
BrotliWriteBits(max_bits as u8, symbols[0], storage_ix, storage);
Expand Down Expand Up @@ -1486,16 +1468,11 @@ fn StoreSimpleHuffmanTree(
BrotliWriteBits(2, 1, storage_ix, storage);
BrotliWriteBits(2, num_symbols.wrapping_sub(1) as u64, storage_ix, storage);
{
for i in 0usize..num_symbols {
let mut j: usize;
j = i.wrapping_add(1);
while j < num_symbols {
{
if (depths[symbols[j]] as i32) < depths[symbols[i]] as i32 {
symbols.swap(j, i);
}
for i in 0..num_symbols {
for j in i + 1..num_symbols {
if depths[symbols[j]] < depths[symbols[i]] {
symbols.swap(j, i);
}
j = j.wrapping_add(1);
}
}
}
Expand Down Expand Up @@ -1705,17 +1682,12 @@ fn StoreTrivialContextMap(
let mut histogram: [u32; 272] = [0; 272];
let mut depths: [u8; 272] = [0; 272];
let mut bits: [u16; 272] = [0; 272];
let mut i: usize;
BrotliWriteBits(1u8, 1u64, storage_ix, storage);
BrotliWriteBits(4u8, repeat_code.wrapping_sub(1) as u64, storage_ix, storage);
histogram[repeat_code] = num_types as u32;
histogram[0] = 1u32;
i = context_bits;
while i < alphabet_size {
{
histogram[i] = 1u32;
}
i = i.wrapping_add(1);
histogram[0] = 1;
for i in context_bits..alphabet_size {
histogram[i] = 1;
}
BuildAndStoreHuffmanTree(
&mut histogram[..],
Expand Down Expand Up @@ -1773,29 +1745,19 @@ fn MoveToFront(v: &mut [u8], index: usize) {
}

fn MoveToFrontTransform(v_in: &[u32], v_size: usize, v_out: &mut [u32]) {
let mut i: usize;
let mut mtf: [u8; 256] = [0; 256];
let mut max_value: u32;
if v_size == 0usize {
return;
}
max_value = v_in[0];
i = 1;
while i < v_size {
{
if v_in[i] > max_value {
max_value = v_in[i];
}
for i in 1..v_size {
if v_in[i] > max_value {
max_value = v_in[i];
}
i = i.wrapping_add(1);
}

i = 0usize;
while i <= max_value as usize {
{
mtf[i] = i as u8;
}
i = i.wrapping_add(1);
for i in 0..=max_value as usize {
mtf[i] = i as u8;
}
{
let mtf_size: usize = max_value.wrapping_add(1) as usize;
Expand Down Expand Up @@ -2129,7 +2091,7 @@ fn StoreSymbolWithContext<Alloc: alloc::Allocator<u8> + alloc::Allocator<u16>>(
}

fn CommandCopyLen(xself: &Command) -> u32 {
xself.copy_len_ & 0x1ffffffu32
xself.copy_len_ & 0x1ffffffu32
}

fn CommandDistanceContext(xself: &Command) -> u32 {
Expand Down
29 changes: 12 additions & 17 deletions src/enc/cluster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,23 +138,18 @@ pub fn BrotliHistogramCombine<
{
/* We maintain a vector of histogram pairs, with the property that the pair
with the maximum bit cost reduction is the first. */
for idx1 in 0usize..num_clusters {
let mut idx2: usize;
idx2 = idx1.wrapping_add(1);
while idx2 < num_clusters {
{
BrotliCompareAndPushToQueue(
out,
cluster_size,
clusters[idx1],
clusters[idx2],
max_num_pairs,
scratch_space,
pairs,
&mut num_pairs,
);
}
idx2 = idx2.wrapping_add(1);
for idx1 in 0..num_clusters {
for idx2 in idx1 + 1..num_clusters {
BrotliCompareAndPushToQueue(
out,
cluster_size,
clusters[idx1],
clusters[idx2],
max_num_pairs,
scratch_space,
pairs,
&mut num_pairs,
);
}
}
}
Expand Down
Loading

0 comments on commit 462f73c

Please sign in to comment.