Skip to content

Commit

Permalink
Remove todo by going with new approach to Rc
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher committed Nov 27, 2023
1 parent 9be1d2d commit e96db7a
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 20 deletions.
27 changes: 8 additions & 19 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,11 @@ impl FunctionBuilder {
/// within the given value. If the given value is not an array and does not contain
/// any arrays, this does nothing.
pub(crate) fn increment_array_reference_count(&mut self, value: ValueId) {
// Reference-counted arrays are only needed for Brillig's copy on write optimization.
if self.current_function.runtime() != RuntimeType::Brillig {
return;
}

match self.type_of_value(value) {
Type::Numeric(_) => (),
Type::Function => (),
Expand All @@ -471,26 +476,10 @@ impl FunctionBuilder {
self.increment_array_reference_count(value);
}
}
Type::Array(element_types, length) => {
self.insert_instruction(Instruction::IncrementRc { value }, None);

if element_types.iter().any(|element| element.contains_an_array()) {
for i in 0..length {
let index = self.field_constant(i as u128);
let element_type = element_types[i % element_types.len()].clone();
let element = self.insert_array_get(value, index, element_type);
self.increment_array_reference_count(element);
}
}
}
Type::Slice(element_types) => {
Type::Array(..) | Type::Slice(..) => {
self.insert_instruction(Instruction::IncrementRc { value }, None);

for element_type in element_types.iter() {
if element_type.contains_an_array() {
todo!("inc_rc for nested slices")
}
}
// If there are nested arrays or slices, we wait until ArrayGet
// is issued to increment the count of that array.
}
}
}
Expand Down
16 changes: 15 additions & 1 deletion compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,13 @@ impl<'a> FunctionContext<'a> {
for element in elements {
element.for_each(|element| {
let element = element.eval(self);

// If we're referencing a sub-array in a larger nested array we need to
// increase the reference count of the sub array. This maintains a
// pessimistic reference count (since some are likely moved rather than shared)
// which is important for Brillig's copy on write optimization. This has no
// effect in ACIR code.
self.builder.increment_array_reference_count(element);
array.push_back(element);
});
}
Expand Down Expand Up @@ -301,6 +308,7 @@ impl<'a> FunctionContext<'a> {
} else {
(array_or_slice[0], None)
};

self.codegen_array_index(
array,
index_value,
Expand Down Expand Up @@ -345,7 +353,13 @@ impl<'a> FunctionContext<'a> {
}
_ => unreachable!("must have array or slice but got {array_type}"),
}
self.builder.insert_array_get(array, offset, typ).into()

// Reference counting in brillig relies on us incrementing reference
// counts when nested arrays/slices are constructed or indexed. This
// has not effect in ACIR code.
let result = self.builder.insert_array_get(array, offset, typ);
self.builder.increment_array_reference_count(result);
result.into()
}))
}

Expand Down

0 comments on commit e96db7a

Please sign in to comment.