Skip to content
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ce13374
fill internal slices pass
vezenovm Oct 23, 2023
554b8a9
cleanup
vezenovm Oct 23, 2023
c2d7ce7
cleanup
vezenovm Oct 23, 2023
27e0835
fix up acir gen to not use value ids with slice values
vezenovm Oct 23, 2023
77b394e
fix for terminator in fill internal slices path
vezenovm Oct 23, 2023
3850cf1
Update compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
TomAFrench Oct 23, 2023
db60a52
working nested array get/set w/ no regressions
vezenovm Oct 25, 2023
8cc5993
Merge branch 'mv/fill-slices-pass' into mv/fill-slices-pass-dbg
vezenovm Oct 25, 2023
a8d0431
working basic slice intrinsics on nested slices
vezenovm Oct 26, 2023
f9b8544
debugging changes for no regressions and changes to ACIR gen
vezenovm Oct 26, 2023
00a2f47
remove gate regression to get slice push back working for nested slices
vezenovm Oct 26, 2023
43f11e5
PR comment on removing from slice
vezenovm Oct 26, 2023
0d39a6b
improve fill internal slices pass
vezenovm Oct 27, 2023
6beabc2
cargo fmt
vezenovm Oct 27, 2023
42c0547
included simple unit test to fill internal slices pass
vezenovm Oct 30, 2023
5236691
cleanup
vezenovm Oct 30, 2023
c31906d
missed cargo fmt
vezenovm Oct 30, 2023
72a8a2a
spell check
vezenovm Oct 30, 2023
f231731
remove broken push back
vezenovm Oct 30, 2023
3b664a4
Update compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
vezenovm Oct 31, 2023
0036938
Merge branch 'mv/slice-struct-fields' into mv/fill-slices-pass
TomAFrench Oct 31, 2023
b48fb85
chore: nargo fmt
TomAFrench Oct 31, 2023
4612c96
Update compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
vezenovm Oct 31, 2023
b59f8ce
Update compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
vezenovm Oct 31, 2023
711a3ff
Update compiler/noirc_evaluator/src/ssa/opt/fill_internal_slices.rs
vezenovm Oct 31, 2023
2a84478
flip order of test data for readability
vezenovm Oct 31, 2023
0bafe8d
format
vezenovm Oct 31, 2023
a4ef11b
reduce nesting in attach_slice_dummies
vezenovm Oct 31, 2023
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
12 changes: 12 additions & 0 deletions compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ pub(crate) fn optimize_into_acir(
print_brillig_trace: bool,
) -> Result<GeneratedAcir, RuntimeError> {
let abi_distinctness = program.return_distinctness;

let ssa = SsaBuilder::new(program, print_ssa_passes)
.run_pass(Ssa::defunctionalize, "After Defunctionalization:")
.run_pass(Ssa::inline_functions, "After Inlining:")
Expand All @@ -60,6 +61,13 @@ pub(crate) fn optimize_into_acir(
.finish();

let brillig = ssa.to_brillig(print_brillig_trace);

// Split off any passes the are not necessary for Brillig generation but are necessary for ACIR generation.
// We only need to fill out nested slices as we need to have a known length when dealing with memory operations
// in ACIR gen while this is not necessary in the Brillig IR.
let ssa = SsaBuilder::new_with_generated_ssa(ssa, print_ssa_passes)
.run_pass(Ssa::fill_internal_slices, "After Fill Internal Slice Dummy Data:")
.finish();
Comment thread
jfecher marked this conversation as resolved.
Outdated
let last_array_uses = ssa.find_last_array_uses();
ssa.into_acir(brillig, abi_distinctness, &last_array_uses)
}
Expand Down Expand Up @@ -133,6 +141,10 @@ impl SsaBuilder {
SsaBuilder { print_ssa_passes, ssa: ssa_gen::generate_ssa(program) }.print("Initial SSA:")
}

fn new_with_generated_ssa(ssa: Ssa, print_ssa_passes: bool) -> SsaBuilder {
SsaBuilder { ssa, print_ssa_passes }
}

fn finish(self) -> Ssa {
self.ssa
}
Expand Down
125 changes: 79 additions & 46 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,9 @@ impl Context {
let rhs_var = read_from_index(rhs_block_id, i)?;
Ok((lhs_var, rhs_var))
}),
_ => unreachable!("ICE: lhs and rhs should be of the same type"),
_ => {
unreachable!("ICE: lhs and rhs should be of the same type")
}
}
}

Expand Down Expand Up @@ -501,7 +503,7 @@ impl Context {
self.initialize_array(block_id, len, Some(output.clone()))?;
}
AcirValue::DynamicArray(_) => {
unreachable!("The output from an intrinsic call is expected to be a single value or an array but got {output:?}");
// Do nothing as a dynamic array returned from a slice intrinsic should already be initialized
}
AcirValue::Var(_, _) => {
// Do nothing
Expand Down Expand Up @@ -737,11 +739,9 @@ impl Context {

let mut dummy_predicate_index = predicate_index;
// We must setup the dummy value to match the type of the value we wish to store
let mut slice_sizes = if store_type.contains_slice_element() {
let store_id = dfg.resolve(store);

self.compute_slice_sizes(store_id, None, dfg);
if let Some(slice_sizes) = self.slice_sizes.get_mut(&store) {
let slice_sizes = if store_type.contains_slice_element() {
self.compute_slice_sizes(store, None, dfg);
if let Some(slice_sizes) = self.slice_sizes.get(&store) {
slice_sizes.clone()
} else {
return Err(InternalError::UnExpected {
Expand All @@ -754,11 +754,12 @@ impl Context {
} else {
vec![]
};

let dummy = self.array_get_value(
&store_type,
block_id,
&mut dummy_predicate_index,
&mut slice_sizes,
&slice_sizes,
)?;

Some(self.convert_array_set_store_value(&store_value, &dummy)?)
Expand Down Expand Up @@ -861,23 +862,20 @@ impl Context {
let res_typ = dfg.type_of_value(results[0]);

let value = if !res_typ.contains_slice_element() {
let mut slice_sizes = vec![];
self.array_get_value(&res_typ, block_id, &mut var_index, &mut slice_sizes)?
self.array_get_value(&res_typ, block_id, &mut var_index, &[])?
} else {
let mut slice_sizes = self
.slice_sizes
.get(&array_id)
.expect("ICE: Array with slices should have associated slice sizes")
.clone();
slice_sizes.drain(0..1);

let result_slice_sizes = slice_sizes.clone();
slice_sizes.remove(0);

let value =
self.array_get_value(&res_typ, block_id, &mut var_index, &mut slice_sizes)?;
let value = self.array_get_value(&res_typ, block_id, &mut var_index, &slice_sizes)?;

// Insert the resulting slice sizes
self.slice_sizes.insert(results[0], result_slice_sizes);
self.slice_sizes.insert(results[0], slice_sizes);

value
};
Expand All @@ -892,7 +890,7 @@ impl Context {
ssa_type: &Type,
block_id: BlockId,
var_index: &mut AcirVar,
slice_sizes: &mut Vec<usize>,
slice_sizes: &[usize],
) -> Result<AcirValue, RuntimeError> {
let one = self.acir_context.add_constant(FieldElement::one());
match ssa_type.clone() {
Expand Down Expand Up @@ -924,16 +922,14 @@ impl Context {
// It is not enough to execute this loop and simply pass the size from the parent definition.
// We need the internal sizes of each type in case of a nested slice.
let mut values = Vector::new();
let current_size = slice_sizes[0];
slice_sizes.drain(0..1);
for _ in 0..current_size {

let (current_size, new_sizes) =
slice_sizes.split_first().expect("should be able to split");

for _ in 0..*current_size {
for typ in element_types.as_ref() {
values.push_back(self.array_get_value(
typ,
block_id,
var_index,
slice_sizes,
)?);
values
.push_back(self.array_get_value(typ, block_id, var_index, new_sizes)?);
}
}
Ok(AcirValue::Array(values))
Expand Down Expand Up @@ -1022,7 +1018,8 @@ impl Context {
}
}

let element_type_sizes = self.init_element_type_sizes_array(&array_typ, array_id, dfg)?;
let element_type_sizes =
self.init_element_type_sizes_array(&array_typ, array_id, None, dfg)?;
let result_value = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: array_len,
Expand Down Expand Up @@ -1109,6 +1106,7 @@ impl Context {
&mut self,
array_typ: &Type,
array_id: ValueId,
array_acir_value: Option<AcirValue>,
dfg: &DataFlowGraph,
) -> Result<BlockId, RuntimeError> {
let element_type_sizes = self.internal_block_id(&array_id);
Expand Down Expand Up @@ -1136,7 +1134,11 @@ impl Context {
Value::Instruction { .. } | Value::Param { .. } => {
// An instruction representing the slice means it has been processed previously during ACIR gen.
// Use the previously defined result of an array operation to fetch the internal type information.
let array_acir_value = self.convert_value(array_id, dfg);
let array_acir_value = if let Some(array_acir_value) = array_acir_value {
array_acir_value
} else {
self.convert_value(array_id, dfg)
};
match array_acir_value {
AcirValue::DynamicArray(AcirDynamicArray {
element_type_sizes: inner_elem_type_sizes,
Expand Down Expand Up @@ -1280,7 +1282,8 @@ impl Context {
var_index: AcirVar,
dfg: &DataFlowGraph,
) -> Result<AcirVar, RuntimeError> {
let element_type_sizes = self.init_element_type_sizes_array(array_typ, array_id, dfg)?;
let element_type_sizes =
self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?;

let true_pred =
self.acir_context.mul_var(var_index, self.current_side_effects_enabled_var)?;
Expand Down Expand Up @@ -1741,26 +1744,50 @@ impl Context {
}
Intrinsic::SlicePushBack => {
let slice_length = self.convert_value(arguments[0], dfg).into_var()?;
let (array_id, array_typ, _) =
self.check_array_is_initialized(arguments[1], dfg)?;
let slice = self.convert_value(arguments[1], dfg);
// TODO(#2461): make sure that we have handled nested struct inputs
let element = self.convert_value(arguments[2], dfg);

// TODO(#3364): make sure that we have handled nested struct inputs
let element = self.convert_value(arguments[2], dfg);
let one = self.acir_context.add_constant(FieldElement::one());
let new_slice_length = self.acir_context.add_var(slice_length, one)?;

// We attach the element no matter what in case len == capacity, as otherwise we
// may get an out of bounds error.
let mut new_slice = Vector::new();
self.slice_intrinsic_input(&mut new_slice, slice)?;
new_slice.push_back(element);

Ok(vec![
AcirValue::Var(new_slice_length, AcirType::field()),
AcirValue::Array(new_slice),
])
self.slice_intrinsic_input(&mut new_slice, slice.clone())?;
new_slice.push_back(element.clone());

// TODO(#3364): This works for non-nested outputs
let len = Self::flattened_value_size(&slice);
let new_elem_size = Self::flattened_value_size(&element);
let new_slice_val = AcirValue::Array(new_slice);
let result_block_id = self.block_id(&result_ids[1]);
self.initialize_array(
result_block_id,
len + new_elem_size,
Some(new_slice_val.clone()),
)?;
let mut var_index = slice_length;
self.array_set_value(element, result_block_id, &mut var_index)?;

let result = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: len + new_elem_size,
element_type_sizes: self.init_element_type_sizes_array(
&array_typ,
array_id,
Some(new_slice_val),
dfg,
)?,
});
Ok(vec![AcirValue::Var(new_slice_length, AcirType::field()), result])
}
Intrinsic::SlicePushFront => {
let slice_length = self.convert_value(arguments[0], dfg).into_var()?;
let slice = self.convert_value(arguments[1], dfg);
// TODO(#2461): make sure that we have handled nested struct inputs
let slice: AcirValue = self.convert_value(arguments[1], dfg);
// TODO(#3364): make sure that we have handled nested struct inputs
let element = self.convert_value(arguments[2], dfg);

let one = self.acir_context.add_constant(FieldElement::one());
Expand All @@ -1782,12 +1809,18 @@ impl Context {
let one = self.acir_context.add_constant(FieldElement::one());
let new_slice_length = self.acir_context.sub_var(slice_length, one)?;

let (_, _, block_id) = self.check_array_is_initialized(arguments[1], dfg)?;
let mut var_index = new_slice_length;
let elem = self.array_get_value(
&dfg.type_of_value(result_ids[2]),
block_id,
&mut var_index,
&[],
)?;

// TODO(#3364): make sure that we have handled nested struct inputs
let mut new_slice = Vector::new();
self.slice_intrinsic_input(&mut new_slice, slice)?;
// TODO(#2461): make sure that we have handled nested struct inputs
let elem = new_slice
.pop_back()
.expect("There are no elements in this slice to be removed");

Ok(vec![
AcirValue::Var(new_slice_length, AcirType::field()),
Expand All @@ -1804,7 +1837,7 @@ impl Context {

let mut new_slice = Vector::new();
self.slice_intrinsic_input(&mut new_slice, slice)?;
// TODO(#2461): make sure that we have handled nested struct inputs
// TODO(#3364): make sure that we have handled nested struct inputs
let elem = new_slice
.pop_front()
.expect("There are no elements in this slice to be removed");
Expand Down Expand Up @@ -1844,7 +1877,7 @@ impl Context {
// they are attempting to insert at too large of an index.
// This check prevents a panic inside of the im::Vector insert method.
if index <= new_slice.len() {
// TODO(#2461): make sure that we have handled nested struct inputs
// TODO(#3364): make sure that we have handled nested struct inputs
new_slice.insert(index, element);
}

Expand Down Expand Up @@ -1882,7 +1915,7 @@ impl Context {
// they are attempting to remove at too large of an index.
// This check prevents a panic inside of the im::Vector remove method.
let removed_elem = if index < new_slice.len() {
// TODO(#2461): make sure that we have handled nested struct inputs
// TODO(#3364): make sure that we have handled nested struct inputs
new_slice.remove(index)
} else {
// This is a dummy value which should never be used if the appropriate
Expand Down
Loading