From 36844fa8fa3b6ec32d918152fda73eb5fb4df1a1 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 1 Dec 2023 15:21:18 +0000 Subject: [PATCH] chore: SliceRemove minor optimization (#3652) # Description ## Problem\* Resolves this discussion https://github.com/noir-lang/noir/pull/3617#discussion_r1411112354 ## 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 Co-authored-by: jfecher --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 64 ++++++++++--------- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index c41674778ad..186e260a52a 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -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 { @@ -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, - ¤t_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, + ¤t_index, + &new_value, + )?; + }; } let new_slice_val = AcirValue::Array(new_slice);