Skip to content

feat: omit element size array for more array types #8257

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

Merged
TomAFrench merged 6 commits into from
Apr 29, 2025
Merged

feat: omit element size array for more array types #8257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
99d84aa
chore: refactor array reading in ACIRgen
Apr 28, 2025
c4bc558
chore: more refactoring
Apr 28, 2025
7f0f11c
chore: add new helper function
Apr 28, 2025
ec6aa46
.
Apr 28, 2025
3fcf43d
feat: omit element size array for more array types
Apr 28, 2025
d36e18c
Merge branch 'master' into tf/omit-element-size-array-for-nested-arrays
TomAFrench Apr 29, 2025
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
28 changes: 20 additions & 8 deletions compiler/noirc_evaluator/src/acir/arrays.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl Context<'_> {
// cf. https://github.com/noir-lang/noir/pull/4971
// For simplicity we compute the offset only for simple arrays
let is_simple_array = dfg.instruction_results(instruction).len() == 1
&& can_omit_element_sizes_array(&array_typ);
&& (array_has_constant_element_size(&array_typ) == Some(1));
let offset = if is_simple_array {
let result_type = dfg.type_of_value(dfg.instruction_results(instruction)[0]);
match array_typ {
Expand Down Expand Up @@ -529,7 +529,7 @@ impl Context<'_> {

self.array_set_value(&store_value, result_block_id, &mut var_index)?;

let element_type_sizes = if !can_omit_element_sizes_array(&array_typ) {
let element_type_sizes = if array_has_constant_element_size(&array_typ).is_none() {
let acir_value = self.convert_value(array, dfg);
Some(self.init_element_type_sizes_array(&array_typ, array, Some(&acir_value), dfg)?)
} else {
Expand Down Expand Up @@ -748,7 +748,10 @@ impl Context<'_> {
var_index: AcirVar,
dfg: &DataFlowGraph,
) -> Result<AcirVar, RuntimeError> {
if !can_omit_element_sizes_array(array_typ) {
if let Some(step_size) = array_has_constant_element_size(array_typ) {
let step_size = self.acir_context.add_constant(step_size);
self.acir_context.mul_var(var_index, step_size)
} else {
let element_type_sizes =
self.init_element_type_sizes_array(array_typ, array_id, None, dfg)?;

Expand All @@ -758,8 +761,6 @@ impl Context<'_> {
self.acir_context
.read_from_memory(element_type_sizes, &predicate_index)
.map_err(RuntimeError::from)
} else {
Ok(var_index)
}
}

Expand Down Expand Up @@ -877,12 +878,23 @@ pub(super) fn flattened_value_size(value: &AcirValue) -> usize {
size
}

// We can omit the element size array for arrays which don't contain arrays or slices.
pub(super) fn can_omit_element_sizes_array(array_typ: &Type) -> bool {
/// Returns whether the array's elements have a constant size.
///
/// This is useful as it then allows us to calculate the flattened index by multiplying by this constant
/// size.
///
/// # Returns
///
/// If the array's element types are all the same size then `array_has_constant_element_size` will return
/// `Some(element_size)` where `element_size` is the size of `array`'s elements. Otherwise returns `None`.
pub(super) fn array_has_constant_element_size(array_typ: &Type) -> Option<u32> {
let types = match array_typ {
Type::Array(types, _) | Type::Slice(types) => types,
_ => panic!("ICE: expected array or slice type"),
};

!types.iter().any(|typ| typ.contains_an_array())
let mut element_sizes = types.iter().map(|typ| typ.flattened_size());
let element_size = element_sizes.next().expect("must have at least one element");

if element_sizes.all(|size| size == element_size) { Some(element_size) } else { None }
}
85 changes: 44 additions & 41 deletions compiler/noirc_evaluator/src/acir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
//! ACIR generation is performed by calling the [Ssa::into_acir] method, providing any necessary brillig bytecode.
//! The compiled program will be returned as an [`Artifacts`] type.

use arrays::can_omit_element_sizes_array;
use fxhash::FxHashMap as HashMap;
use noirc_errors::call_stack::CallStack;
use std::collections::{BTreeMap, HashSet};
Expand Down Expand Up @@ -1327,16 +1326,17 @@ impl<'a> Context<'a> {
self.array_set_value(&element, result_block_id, &mut var_index)?;
}

let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&new_slice_val),
dfg,
)?)
} else {
None
};
let element_type_sizes =
if arrays::array_has_constant_element_size(&slice_typ).is_none() {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&new_slice_val),
dfg,
)?)
} else {
None
};

let value_types = new_slice_val.flat_numeric_types();
assert_eq!(
Expand Down Expand Up @@ -1389,16 +1389,17 @@ impl<'a> Context<'a> {
Some(new_slice_val.clone()),
)?;

let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&new_slice_val),
dfg,
)?)
} else {
None
};
let element_type_sizes =
if arrays::array_has_constant_element_size(&slice_typ).is_none() {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&new_slice_val),
dfg,
)?)
} else {
None
};

let value_types = new_slice_val.flat_numeric_types();
assert_eq!(
Expand Down Expand Up @@ -1606,16 +1607,17 @@ impl<'a> Context<'a> {
}
}

let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&slice),
dfg,
)?)
} else {
None
};
let element_type_sizes =
if arrays::array_has_constant_element_size(&slice_typ).is_none() {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&slice),
dfg,
)?)
} else {
None
};

let value_types = slice.flat_numeric_types();
assert_eq!(
Expand Down Expand Up @@ -1732,16 +1734,17 @@ impl<'a> Context<'a> {
}

let new_slice_val = AcirValue::Array(new_slice);
let element_type_sizes = if !can_omit_element_sizes_array(&slice_typ) {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&new_slice_val),
dfg,
)?)
} else {
None
};
let element_type_sizes =
if arrays::array_has_constant_element_size(&slice_typ).is_none() {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(&new_slice_val),
dfg,
)?)
} else {
None
};

let value_types = new_slice_val.flat_numeric_types();
assert_eq!(
Expand Down
4 changes: 2 additions & 2 deletions ..._composite_calldata/execute__tests__force_brillig_false_inliner_-9223372036854775808.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions ...ion_success/databus_composite_calldata/execute__tests__force_brillig_false_inliner_0.snap
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading