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.

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

fix: Attach types to Dynamic Memory #4363

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Empty file modified CHANGELOG.md
100644 → 100755
Empty file.
Empty file modified CONTRIBUTING.md
100644 → 100755
Empty file.
Empty file modified Cargo.lock
100644 → 100755
Empty file.
Empty file modified Cargo.toml
100644 → 100755
Empty file.
Empty file modified Dockerfile
100644 → 100755
Empty file.
Empty file modified Dockerfile.ci
100644 → 100755
Empty file.
Empty file modified Dockerfile.packages
100644 → 100755
Empty file.
Empty file modified LICENSE-APACHE
100644 → 100755
Empty file.
Empty file modified LICENSE-MIT
100644 → 100755
Empty file.
Empty file modified README.md
100644 → 100755
Empty file.
Empty file modified SUPPORT.md
100644 → 100755
Empty file.
17 changes: 15 additions & 2 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ impl AcirType {
pub(crate) fn unsigned(bit_size: u32) -> Self {
AcirType::NumericType(NumericType::Unsigned { bit_size })
}

pub(crate) fn to_numeric_type(self) -> NumericType {
match self {
AcirType::NumericType(numeric_type) => numeric_type,
AcirType::Array(_, _) => unreachable!("cannot fetch a numeric type for an array type"),
}
}
}

impl From<SsaType> for AcirType {
Expand All @@ -88,6 +95,12 @@ impl<'a> From<&'a SsaType> for AcirType {
}
}

impl From<NumericType> for AcirType {
fn from(value: NumericType) -> Self {
AcirType::NumericType(value)
}
}

#[derive(Debug, Default)]
/// Context object which holds the relationship between
/// `Variables`(AcirVar) and types such as `Expression` and `Witness`
Expand Down Expand Up @@ -1415,13 +1428,13 @@ impl AcirContext {
}
Ok(values)
}
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }) => {
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, value_types, .. }) => {
try_vecmap(0..len, |i| {
let index_var = self.add_constant(i);

Ok::<(AcirVar, AcirType), InternalError>((
self.read_from_memory(block_id, &index_var)?,
AcirType::field(),
value_types[i].into(),
))
})
}
Expand Down
71 changes: 59 additions & 12 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,13 @@ pub(crate) struct AcirDynamicArray {
block_id: BlockId,
/// Length of the array
len: usize,
/// An ACIR dynamic array is a flat structure, so we use
/// the inner structure of an `AcirType::NumericType` directly.
/// Some usages of ACIR arrays (e.g. black box hashes) require the bit size
/// of every value to be known, thus we store the types as part of the dynamic
/// array definition.
/// The length of the value types vector must match the `len` field in this structure.
value_types: Vec<NumericType>,
/// Identification for the ACIR dynamic array
/// inner element type sizes array
element_type_sizes: Option<BlockId>,
Expand Down Expand Up @@ -1007,15 +1014,32 @@ impl Context {
} else {
None
};

let array_acir_value = self.convert_value(array_id, dfg);
let value_types = Self::dynamic_array_value_types(array_acir_value);
// Compiler sanity check
assert!(value_types.len() == array_len, "ICE: The length of the flattened type array should match the length of the dynamic array");

let result_value = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: array_len,
value_types,
element_type_sizes,
});
self.define_result(dfg, instruction, result_value);
Ok(())
}

fn dynamic_array_value_types(
array_acir_value: AcirValue,
) -> Vec<NumericType> {
match array_acir_value {
AcirValue::Array(_) => array_acir_value.flatten().into_iter().map(|(_, typ)| typ.to_numeric_type()).collect::<Vec<_>>(),
AcirValue::DynamicArray(AcirDynamicArray { value_types, .. }) => value_types,
_ => unreachable!("An AcirValue::Var cannot be used as an array value"),
}
}

fn array_set_value(
&mut self,
value: &AcirValue,
Expand Down Expand Up @@ -1093,7 +1117,7 @@ impl Context {
&mut self,
array_typ: &Type,
array_id: ValueId,
array_acir_value: Option<AcirValue>,
supplied_array_value: Option<&AcirValue>,
dfg: &DataFlowGraph,
) -> Result<BlockId, RuntimeError> {
let element_type_sizes = self.internal_block_id(&array_id);
Expand All @@ -1119,10 +1143,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 = if let Some(array_acir_value) = array_acir_value {
let value = &self.convert_value(array_id, dfg);
let array_acir_value = if let Some(array_acir_value) = supplied_array_value {
array_acir_value
} else {
self.convert_value(array_id, dfg)
value
};
match array_acir_value {
AcirValue::DynamicArray(AcirDynamicArray {
Expand All @@ -1138,7 +1163,7 @@ impl Context {
}
)?;
self.copy_dynamic_array(
inner_elem_type_sizes,
*inner_elem_type_sizes,
element_type_sizes,
type_sizes_array_len,
)?;
Expand Down Expand Up @@ -1192,7 +1217,7 @@ impl Context {
}
}

// The final array should will the flattened index at each outer array index
// The final array should be the flattened index of each element of the outer array
let init_values = vecmap(flat_elem_type_sizes, |type_size| {
let var = self.acir_context.add_constant(type_size);
AcirValue::Var(var, AcirType::field())
Expand Down Expand Up @@ -1600,7 +1625,9 @@ impl Context {
}
}

let inputs = vecmap(&arguments_no_slice_len, |arg| self.convert_value(*arg, dfg));
let inputs = vecmap(&arguments_no_slice_len, |arg| {
self.convert_value(*arg, dfg)
});

let output_count = result_ids.iter().fold(0usize, |sum, result_id| {
sum + dfg.try_get_array_length(*result_id).unwrap_or(1)
Expand Down Expand Up @@ -1683,15 +1710,20 @@ impl Context {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(new_slice_val),
Some(&new_slice_val),
dfg,
)?)
} else {
None
};

let value_types = Self::dynamic_array_value_types(new_slice_val);
assert!(value_types.len() == new_elem_size, "ICE: Value types array must match new slice size");

let result = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: new_elem_size,
value_types,
element_type_sizes,
});
Ok(vec![AcirValue::Var(new_slice_length, AcirType::field()), result])
Expand Down Expand Up @@ -1738,15 +1770,20 @@ impl Context {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(new_slice_val),
Some(&new_slice_val),
dfg,
)?)
} else {
None
};

let value_types = Self::dynamic_array_value_types(new_slice_val);
assert!(value_types.len() == new_slice_size, "ICE: Value types array must match new slice size");

let result = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: new_slice_size,
value_types,
element_type_sizes,
});

Expand Down Expand Up @@ -1943,15 +1980,20 @@ impl Context {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(slice),
Some(&slice),
dfg,
)?)
} else {
None
};

let value_types = Self::dynamic_array_value_types(slice);
assert!(value_types.len() == slice_size, "ICE: Value types array must match new slice size");

let result = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: slice_size,
value_types,
element_type_sizes,
});

Expand Down Expand Up @@ -2059,15 +2101,20 @@ impl Context {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(new_slice_val),
Some(&new_slice_val),
dfg,
)?)
} else {
None
};

let value_types = Self::dynamic_array_value_types(new_slice_val);
assert!(value_types.len() == slice_size, "ICE: Value types array must match new slice size");

let result = AcirValue::DynamicArray(AcirDynamicArray {
block_id: result_block_id,
len: slice_size,
value_types,
element_type_sizes,
});

Expand All @@ -2094,14 +2141,14 @@ impl Context {
self.slice_intrinsic_input(old_slice, var)?;
}
}
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, .. }) => {
AcirValue::DynamicArray(AcirDynamicArray { block_id, len, value_types, .. }) => {
for i in 0..len {
// We generate witnesses corresponding to the array values
let index_var = self.acir_context.add_constant(i);

let value_read_var =
self.acir_context.read_from_memory(block_id, &index_var)?;
let value_read = AcirValue::Var(value_read_var, AcirType::field());
let value_read = AcirValue::Var(value_read_var, value_types[i].into());

old_slice.push_back(value_read);
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ impl Instruction {
// In ACIR, a division with a false predicate outputs (0,0), so it cannot replace another instruction unless they have the same predicate
bin.operator != BinaryOp::Div
}
Cast(_, _) | Truncate { .. } | Not(_) | ArrayGet { .. } | ArraySet { .. } => true,
// Cast(_, _) | Truncate { .. } | Not(_) | ArrayGet { .. } | ArraySet { .. } => true,
Cast(_, _) | Truncate { .. } | Not(_) | ArrayGet { .. } | ArraySet { .. } => true,

// These either have side-effects or interact with memory
Constrain(..)
Expand Down Expand Up @@ -277,7 +278,7 @@ impl Instruction {
| Not(_)
| Truncate { .. }
| Allocate
| Load { .. }
| Load { .. }
| ArrayGet { .. }
| ArraySet { .. } => false,

Expand Down
Empty file modified cspell.json
100644 → 100755
Empty file.
Empty file modified default.nix
100644 → 100755
Empty file.
Empty file modified deny.toml
100644 → 100755
Empty file.
Empty file modified flake.lock
100644 → 100755
Empty file.
Empty file modified flake.nix
100644 → 100755
Empty file.
Empty file modified package.json
100644 → 100755
Empty file.
Empty file modified release-please-config.json
100644 → 100755
Empty file.
Empty file modified rust-toolchain.toml
100644 → 100755
Empty file.
Empty file modified shell.nix
100644 → 100755
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "array_dynamic_blackbox_input"
type = "bin"
authors = [""]
compiler_version = ">=0.24.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
index = "1"
leaf = ["51", "109", "224", "175", "60", "42", "79", "222", "117", "255", "174", "79", "126", "242", "74", "34", "100", "35", "20", "200", "109", "89", "191", "219", "41", "10", "118", "217", "165", "224", "215", "109"]
path = ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", "23", "24", "25", "26", "27", "28", "29", "30", "31", "32", "33", "34", "35", "36", "37", "38", "39", "40", "41", "42", "43", "44", "45", "46", "47", "48", "49", "50", "51", "52", "53", "54", "55", "56", "57", "58", "59", "60", "61", "62", "63"]
root = [79, 230, 126, 184, 98, 125, 226, 58, 117, 45, 140, 15, 72, 118, 89, 173, 117, 161, 166, 0, 214, 125, 13, 16, 113, 81, 173, 156, 97, 15, 57, 216]
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
fn main(leaf: [u8; 32], path: [u8; 64], index: u32, root: [u8; 32]) {
compute_root(leaf, path, index, root);
}

fn compute_root(leaf: [u8; 32], path: [u8; 64], _index: u32, root: [u8; 32]) {
let mut current = leaf;
let mut index = _index;

for i in 0..2 {
let mut hash_input = [0; 64];
let offset = i * 32;
let is_right = (index & 1) != 0;
let a = if is_right { 32 } else { 0 };
let b = if is_right { 0 } else { 32 };

for j in 0..32 {
hash_input[j + a] = current[j];
hash_input[j + b] = path[offset + j];
}

current = dep::std::hash::sha256(hash_input);
index = index >> 1;
}

// Regression for issue #4258
assert(root == current);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ struct Foo {
}

struct FooParent {
array: [Field; 3],
array: [u8; 3],
foos: [Foo; 4],
}

Expand Down
Empty file modified wasm-bindgen-cli.nix
100644 → 100755
Empty file.
Empty file modified yarn.lock
100644 → 100755
Empty file.
Loading