Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(opt): Reduce array cloning by initializing refcounts to 0, not 1 #6577

Closed
wants to merge 11 commits into from
Closed

feat(opt): Reduce array cloning by initializing refcounts to 0, not 1 #6577

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5e04067
Avoid incrementing reference counts in some cases
jfecher Nov 20, 2024
d2a7849
Move inc_rc 2 instructions earlier
jfecher Nov 20, 2024
65054d6
Trigger CI
jfecher Nov 20, 2024
3e7d5f3
Merge branch 'master' into jf/rearrange-inc-rcs
jfecher Nov 20, 2024
97cf674
Trigger CI, remove external repos tag since noir-edwards needs an update
jfecher Nov 20, 2024
1b43b06
Merge branch 'jf/rearrange-inc-rcs' of https://github.com/noir-lang/n…
jfecher Nov 20, 2024
4c234cf
Update compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
jfecher Nov 20, 2024
b4f0187
Remove need for extra parameter
jfecher Nov 21, 2024
204de4a
Start rc at 0, not 1
jfecher Nov 21, 2024
1e12188
added argument for initial rc
sirasistant Nov 21, 2024
7877fd6
Merge branch 'master' into jf/init-rc-to-zero
jfecher Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ impl<'block> BrilligBlock<'block> {
destination_vector,
source_size_register,
None,
None,
);

// Items
Expand Down Expand Up @@ -772,13 +773,14 @@ impl<'block> BrilligBlock<'block> {
// Initialize the variable
match new_variable {
BrilligVariable::BrilligArray(brillig_array) => {
self.brillig_context.codegen_initialize_array(brillig_array);
self.brillig_context.codegen_initialize_array(brillig_array, None);
}
BrilligVariable::BrilligVector(vector) => {
let size = self
.brillig_context
.make_usize_constant_instruction(array.len().into());
self.brillig_context.codegen_initialize_vector(vector, size, None);
self.brillig_context
.codegen_initialize_vector(vector, size, None, None);
self.brillig_context.deallocate_single_addr(size);
}
_ => unreachable!(
Expand Down Expand Up @@ -1773,7 +1775,7 @@ impl<'block> BrilligBlock<'block> {
unreachable!("ICE: allocate_foreign_call_array() expects an array, got {typ:?}")
};

self.brillig_context.codegen_initialize_array(array);
self.brillig_context.codegen_initialize_array(array, None);

let mut index = 0_usize;
for _ in 0..*size {
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
assert!(source_field.bit_size == F::max_num_bits());
assert!(radix.bit_size == 32);

self.codegen_initialize_array(target_array);
self.codegen_initialize_array(target_array, None);

let heap_array = self.codegen_brillig_array_to_heap_array(target_array);

Expand Down
18 changes: 13 additions & 5 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ use super::{
BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
};

const INITIAL_ARRAY_REF_COUNT: usize = 0;

impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<F, Registers> {
/// Allocates an array of size `size` and stores the pointer to the array
/// in `pointer_register`
Expand Down Expand Up @@ -419,12 +421,16 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
}

/// Initializes an array, allocating memory to store its representation and initializing the reference counter.
pub(crate) fn codegen_initialize_array(&mut self, array: BrilligArray) {
pub(crate) fn codegen_initialize_array(
&mut self,
array: BrilligArray,
initial_rc: Option<usize>,
) {
self.codegen_allocate_immediate_mem(array.pointer, array.size + 1);
self.indirect_const_instruction(
array.pointer,
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
1_usize.into(),
initial_rc.unwrap_or(INITIAL_ARRAY_REF_COUNT).into(),
);
}

Expand All @@ -433,12 +439,13 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
vector: BrilligVector,
size: SingleAddrVariable,
capacity: Option<SingleAddrVariable>,
initial_rc: Option<usize>,
) {
// Write RC
self.indirect_const_instruction(
vector.pointer,
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
1_usize.into(),
initial_rc.unwrap_or(INITIAL_ARRAY_REF_COUNT).into(),
);

// Write size
Expand All @@ -459,6 +466,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
vector: BrilligVector,
size: SingleAddrVariable,
capacity: Option<SingleAddrVariable>, // Defaults to size if None
initial_rc: Option<usize>, // Defaults to INITIAL_ARRAY_REF_COUNT if none
) {
let allocation_size = self.allocate_register();
// Allocation size = capacity + 3 (rc, size, capacity)
Expand All @@ -471,7 +479,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
self.codegen_allocate_mem(vector.pointer, allocation_size);
self.deallocate_register(allocation_size);

self.codegen_initialize_vector_metadata(vector, size, capacity);
self.codegen_initialize_vector_metadata(vector, size, capacity, initial_rc);
}

/// We don't know the length of a vector returned externally before the call
Expand All @@ -498,7 +506,7 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
self.indirect_const_instruction(
vector.pointer,
BRILLIG_MEMORY_ADDRESSING_BIT_SIZE,
1_usize.into(),
INITIAL_ARRAY_REF_COUNT.into(),
);

// Initialize size
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
}

for (i, bit_size) in arguments.iter().flat_map(flat_bit_sizes).enumerate() {
// Calldatacopy tags everything with field type, so when downcast when necessary

Check warning on line 171 in compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs

View workflow job for this annotation

GitHub Actions / Code 

Unknown word (Calldatacopy)
if bit_size < F::max_num_bits() {
self.cast_instruction(
SingleAddrVariable::new(
Expand Down Expand Up @@ -199,15 +199,15 @@
let deflattened_items_pointer = if is_vector {
let vector = BrilligVector { pointer: deflattened_array_pointer };

self.codegen_initialize_vector(vector, deflattened_size_variable, None);
self.codegen_initialize_vector(vector, deflattened_size_variable, None, Some(1));

self.codegen_make_vector_items_pointer(vector)
} else {
let arr = BrilligArray {
pointer: deflattened_array_pointer,
size: item_count * item_type.len(),
};
self.codegen_initialize_array(arr);
self.codegen_initialize_array(arr, Some(1));
self.codegen_make_array_items_pointer(arr)
};

Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/prepare_vector_push.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ pub(crate) fn reallocate_vector_for_insertion<
target_vector,
target_size,
Some(source_capacity),
Some(1),
);
}
});
Expand All @@ -201,6 +202,7 @@ pub(crate) fn reallocate_vector_for_insertion<
target_vector,
target_size,
Some(double_size),
Some(1),
);
brillig_context.deallocate_single_addr(double_size);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_pop_back.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub(super) fn compile_vector_pop_back_procedure<F: AcirField + DebugToString>(
brillig_context.codegen_update_vector_length(target_vector, target_size);
} else {
// We need to clone the source vector
brillig_context.codegen_initialize_vector(target_vector, target_size, None);
brillig_context.codegen_initialize_vector(target_vector, target_size, None, Some(1));

let target_vector_items_pointer =
brillig_context.codegen_make_vector_items_pointer(target_vector);
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_pop_front.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,10 @@ pub(super) fn compile_vector_pop_front_procedure<F: AcirField + DebugToString>(
target_vector,
target_size,
Some(source_capacity),
Some(1),
);
} else {
brillig_context.codegen_initialize_vector(target_vector, target_size, None);
brillig_context.codegen_initialize_vector(target_vector, target_size, None, Some(1));

let target_vector_items_pointer =
brillig_context.codegen_make_vector_items_pointer(target_vector);
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/vector_remove.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub(super) fn compile_vector_remove_procedure<F: AcirField + DebugToString>(
brillig_context
.codegen_vector_items_pointer(target_vector, target_vector_items_pointer);
} else {
brillig_context.codegen_initialize_vector(target_vector, target_size, None);
brillig_context.codegen_initialize_vector(target_vector, target_size, None, Some(1));

// Copy the elements to the left of the index
brillig_context
Expand Down
Loading