Skip to content

Commit

Permalink
chore: SliceRemove minor optimization (#3652)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves this discussion
#3617 (comment)

## Summary\*

Reference the discussion and the PR it is under for more context.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
4 people authored Dec 1, 2023
1 parent 5a4a73d commit 36844fa
Showing 1 changed file with 34 additions and 30 deletions.
64 changes: 34 additions & 30 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2128,9 +2128,9 @@ impl Context {
)?;

// Read from the original slice the value we want to insert into our new slice.
// We need to make sure of two things:
// 1. That we read the previous element for when we have an index greater than insertion index.
// 2. That the index we are reading from is within the array bounds
// We need to make sure that we read the previous element when our current index is greater than insertion index.
// If the index for the previous element is out of the array bounds we can avoid the check for whether
// the current index is over the insertion index.
let shifted_index = if i < inner_elem_size_usize {
current_index
} else {
Expand Down Expand Up @@ -2279,41 +2279,45 @@ impl Context {
// can lead to a potential out of bounds error. In this case we just fetch from the original slice
// at the current index. As we are decreasing the slice in length, this is a safe operation.
let result_block_id = self.block_id(&result_ids[1]);
self.initialize_array(result_block_id, slice_size, None)?;
self.initialize_array(
result_block_id,
slice_size,
Some(AcirValue::Array(new_slice.clone())),
)?;
for i in 0..slice_size {
let current_index = self.acir_context.add_constant(i);

let shifted_index = if (i + popped_elements_size) >= slice_size {
current_index
} else {
self.acir_context.add_constant(i + popped_elements_size)
};
let value_current_index = &new_slice[i].borrow_var()?;

let value_shifted_index =
self.acir_context.read_from_memory(block_id, &shifted_index)?;
let value_current_index = new_slice[i].borrow_var()?;
if slice_size > (i + popped_elements_size) {
let shifted_index =
self.acir_context.add_constant(i + popped_elements_size);

let use_shifted_value = self.acir_context.more_than_eq_var(
current_index,
flat_user_index,
64,
self.current_side_effects_enabled_var,
)?;
let value_shifted_index =
self.acir_context.read_from_memory(block_id, &shifted_index)?;

let shifted_value_pred =
self.acir_context.mul_var(value_shifted_index, use_shifted_value)?;
let not_pred = self.acir_context.sub_var(one, use_shifted_value)?;
let current_value_pred =
self.acir_context.mul_var(not_pred, value_current_index)?;
let use_shifted_value = self.acir_context.more_than_eq_var(
current_index,
flat_user_index,
64,
self.current_side_effects_enabled_var,
)?;

let new_value =
self.acir_context.add_var(shifted_value_pred, current_value_pred)?;
let shifted_value_pred =
self.acir_context.mul_var(value_shifted_index, use_shifted_value)?;
let not_pred = self.acir_context.sub_var(one, use_shifted_value)?;
let current_value_pred =
self.acir_context.mul_var(not_pred, *value_current_index)?;

self.acir_context.write_to_memory(
result_block_id,
&current_index,
&new_value,
)?;
let new_value =
self.acir_context.add_var(shifted_value_pred, current_value_pred)?;

self.acir_context.write_to_memory(
result_block_id,
&current_index,
&new_value,
)?;
};
}

let new_slice_val = AcirValue::Array(new_slice);
Expand Down

0 comments on commit 36844fa

Please sign in to comment.