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(acir): Use types on dynamic arrays #4364

Merged
merged 8 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
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
78 changes: 65 additions & 13 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
vezenovm marked this conversation as resolved.
Show resolved Hide resolved
/// 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 @@ -150,6 +157,16 @@ impl AcirValue {
AcirValue::DynamicArray(_) => unimplemented!("Cannot flatten a dynamic array"),
}
}

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

impl Ssa {
Expand Down Expand Up @@ -1007,9 +1024,15 @@ impl Context {
} else {
None
};

let value_types = self.convert_value(array_id, dfg).flat_numeric_types();
// 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);
Expand Down Expand Up @@ -1093,7 +1116,7 @@ impl Context {
&mut self,
array_typ: &Type,
array_id: ValueId,
array_acir_value: Option<AcirValue>,
supplied_acir_value: Option<&AcirValue>,
dfg: &DataFlowGraph,
) -> Result<BlockId, RuntimeError> {
let element_type_sizes = self.internal_block_id(&array_id);
Expand All @@ -1119,26 +1142,23 @@ 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 {
array_acir_value
} else {
self.convert_value(array_id, dfg)
};
let array_acir_value = &self.convert_value(array_id, dfg);
let array_acir_value = supplied_acir_value.unwrap_or(array_acir_value);
match array_acir_value {
AcirValue::DynamicArray(AcirDynamicArray {
element_type_sizes: inner_elem_type_sizes,
..
}) => {
if let Some(inner_elem_type_sizes) = inner_elem_type_sizes {
if self.initialized_arrays.contains(&inner_elem_type_sizes) {
let type_sizes_array_len = self.internal_mem_block_lengths.get(&inner_elem_type_sizes).copied().ok_or_else(||
if self.initialized_arrays.contains(inner_elem_type_sizes) {
let type_sizes_array_len = self.internal_mem_block_lengths.get(inner_elem_type_sizes).copied().ok_or_else(||
InternalError::General {
message: format!("Array {array_id}'s inner element type sizes array does not have a tracked length"),
call_stack: self.acir_context.get_call_stack(),
}
)?;
self.copy_dynamic_array(
inner_elem_type_sizes,
*inner_elem_type_sizes,
element_type_sizes,
type_sizes_array_len,
)?;
Expand Down Expand Up @@ -1683,15 +1703,23 @@ 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 = new_slice_val.flat_numeric_types();
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 +1766,23 @@ 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 = new_slice_val.flat_numeric_types();
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 +1979,23 @@ impl Context {
Some(self.init_element_type_sizes_array(
&slice_typ,
slice_contents,
Some(slice),
Some(&slice),
dfg,
)?)
} else {
None
};

let value_types = slice.flat_numeric_types();
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 +2103,23 @@ 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 = new_slice_val.flat_numeric_types();
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
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);
}
Loading