Skip to content

feat: Change the layout of arrays and vectors to be a single pointer #8448

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 14 commits into from
Sep 11, 2024
+1,021 −994
Merged

feat: Change the layout of arrays and vectors to be a single pointer #8448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
81dc3d0
wip: change layout of arrays and vectors
sirasistant Sep 5, 2024
b23f7e3
fix slice ops
sirasistant Sep 5, 2024
ec5a946
update blackboxes
sirasistant Sep 5, 2024
6627d93
various fixes
sirasistant Sep 6, 2024
399c3ea
tons of fixes, remaning, foreign call slice returns
sirasistant Sep 6, 2024
b2ed449
fix: foreign call slice return
sirasistant Sep 9, 2024
7f85ecc
remove store after increment rc
sirasistant Sep 9, 2024
eb7856f
Merge branch 'master' into arv/array_vector_layout
sirasistant Sep 9, 2024
dc06be8
refactor
sirasistant Sep 9, 2024
97016e0
Merge branch 'master' into arv/array_vector_layout
sirasistant Sep 9, 2024
f8ff875
Merge branch 'master' into arv/array_vector_layout
sirasistant Sep 9, 2024
656981c
fix stack leaks
sirasistant Sep 9, 2024
3f36a2c
docs
sirasistant Sep 9, 2024
a64297f
noir fmt
sirasistant Sep 9, 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
2 changes: 1 addition & 1 deletion noir-projects/noir-protocol-circuits/crates/reset-kernel-lib/src/lib.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ pub use reset::{
key_validation_hint::KeyValidationHint,
transient_data::{TransientDataIndexHint, verify_squashed_transient_data},
tree_leaf_read_request::TreeLeafReadRequestHint
};
};
3 changes: 1 addition & 2 deletions noir-projects/noir-protocol-circuits/crates/types/src/abis/global_variables.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::{
address::{AztecAddress, EthAddress}, abis::gas_fees::GasFees,
constants::{GLOBAL_VARIABLES_LENGTH},
address::{AztecAddress, EthAddress}, abis::gas_fees::GasFees, constants::GLOBAL_VARIABLES_LENGTH,
traits::{Deserialize, Empty, Serialize}, utils::reader::Reader
};

Expand Down
2 changes: 1 addition & 1 deletion noir-projects/noir-protocol-circuits/crates/types/src/lib.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@ mod data;
mod storage;
mod validate;

pub use abis::kernel_circuit_public_inputs::{KernelCircuitPublicInputs, PrivateKernelCircuitPublicInputs, PublicKernelCircuitPublicInputs};
pub use abis::kernel_circuit_public_inputs::{KernelCircuitPublicInputs, PrivateKernelCircuitPublicInputs, PublicKernelCircuitPublicInputs};
43 changes: 20 additions & 23 deletions noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,17 +449,16 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>> VM<'a, F, B> {
}
HeapValueType::Array { value_types, size } => {
let array_address = self.memory.read_ref(value_address);
let array_start = self.memory.read_ref(array_address);
Copy link
Collaborator

@ludamad ludamad Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, what was it before? we read the array start from the array address?

Copy link
Contributor Author

@sirasistant sirasistant Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, an array with nested arrays had:

stack: parent items ptr, parent RC
parent items ptr: [ptr_a, ptr_b, ...]
ptr_a: a items ptr, a RC
a items ptr: [a_0, a_1, a_2...]

Now an array with nested arrays has

stack: parent ptr
parent ptr: [parent RC, ptr_a, ptr_b, ...]
ptr_a: [a RC, a_0, a_1, a_2...]

Copy link
Contributor Author

@sirasistant sirasistant Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So previously the runtime needed to do a double dereference to get to the items. Now it's a single dereference and add one, to reach the items pointer

Copy link
Contributor Author

@sirasistant sirasistant Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For vectors now it's also very similar:

stack: parent ptr
parent ptr: [parent RC, parent size, ptr_a, ptr_b, ...]
ptr_a: [a RC, a size, a_0, a_1, a_2...]

self.read_slice_of_values_from_memory(array_start, *size, value_types)
let items_start = MemoryAddress(array_address.to_usize() + 1);
self.read_slice_of_values_from_memory(items_start, *size, value_types)
}
HeapValueType::Vector { value_types } => {
let vector_address = self.memory.read_ref(value_address);
let vector_start = self.memory.read_ref(vector_address);
let size_address: MemoryAddress =
(vector_address.to_usize() + 1).into();
let size_address = MemoryAddress(vector_address.to_usize() + 1);
let items_start = MemoryAddress(vector_address.to_usize() + 2);
let vector_size = self.memory.read(size_address).to_usize();
self.read_slice_of_values_from_memory(
vector_start,
items_start,
vector_size,
value_types,
)
Expand Down Expand Up @@ -646,8 +645,8 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>> VM<'a, F, B> {
current_pointer = MemoryAddress(current_pointer.to_usize() + 1);
}
HeapValueType::Array { .. } => {
let destination = self.memory.read_ref(current_pointer);
let destination = self.memory.read_ref(destination);
let destination =
MemoryAddress(self.memory.read_ref(current_pointer).0 + 1);
self.write_slice_of_values_to_memory(
destination,
values,
Expand Down Expand Up @@ -2005,33 +2004,31 @@ mod tests {
vec![MemoryValue::new_field(FieldElement::from(9u128))];

// construct memory by declaring all inner arrays/vectors first
// Declare v2
let v2_ptr: usize = 0usize;
let mut memory = v2.clone();
let v2_start = memory.len();
memory.extend(vec![MemoryValue::from(v2_ptr), v2.len().into(), MemoryValue::from(1_u32)]);
let mut memory = vec![MemoryValue::from(1_u32), v2.len().into()];
memory.extend(v2.clone());
let a4_ptr = memory.len();
memory.extend(vec![MemoryValue::from(1_u32)]);
memory.extend(a4.clone());
let a4_start = memory.len();
memory.extend(vec![MemoryValue::from(a4_ptr), MemoryValue::from(1_u32)]);
let v6_ptr = memory.len();
memory.extend(vec![MemoryValue::from(1_u32), v6.len().into()]);
memory.extend(v6.clone());
let v6_start = memory.len();
memory.extend(vec![MemoryValue::from(v6_ptr), v6.len().into(), MemoryValue::from(1_u32)]);
let a9_ptr = memory.len();
memory.extend(vec![MemoryValue::from(1_u32)]);
memory.extend(a9.clone());
let a9_start = memory.len();
memory.extend(vec![MemoryValue::from(a9_ptr), MemoryValue::from(1_u32)]);
// finally we add the contents of the outer array
let outer_ptr = memory.len();
memory.extend(vec![MemoryValue::from(1_u32)]);
let outer_start = memory.len();
let outer_array = vec![
MemoryValue::new_field(FieldElement::from(1u128)),
MemoryValue::from(v2.len() as u32),
MemoryValue::from(v2_start),
MemoryValue::from(a4_start),
MemoryValue::from(v2_ptr),
MemoryValue::from(a4_ptr),
MemoryValue::new_field(FieldElement::from(5u128)),
MemoryValue::from(v6.len() as u32),
MemoryValue::from(v6_start),
MemoryValue::from(a9_start),
MemoryValue::from(v6_ptr),
MemoryValue::from(a9_ptr),
];
memory.extend(outer_array.clone());

Expand Down Expand Up @@ -2075,7 +2072,7 @@ mod tests {
// input = 0
Opcode::Const {
destination: r_input,
value: (outer_ptr).into(),
value: (outer_start).into(),
bit_size: BitSize::Integer(IntegerBitSize::U32),
},
// some_function(input)
Expand Down
Loading