Skip to content

Commit 8f24751

Browse files
authored
fix(ssa refactor): Implement merging of array values during flattening pass (#1767)
* Merge array values * Remove redundant clone
1 parent b52f25d commit 8f24751

File tree

3 files changed

+68
-11
lines changed

3 files changed

+68
-11
lines changed

crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,8 @@ impl Context {
483483
(Type::Numeric(lhs_type), Type::Numeric(rhs_type)) => {
484484
assert_eq!(
485485
lhs_type, rhs_type,
486-
"lhs and rhs types in a binary operation are always the same"
486+
"lhs and rhs types in {:?} are not the same",
487+
binary
487488
);
488489
Type::Numeric(lhs_type)
489490
}

crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs

+11-8
Original file line numberDiff line numberDiff line change
@@ -277,11 +277,12 @@ impl Instruction {
277277
if let (Some((array, _)), Some(index)) = (array, index) {
278278
let index =
279279
index.try_to_u64().expect("Expected array index to fit in u64") as usize;
280-
assert!(index < array.len());
281-
SimplifiedTo(array[index])
282-
} else {
283-
None
280+
281+
if index < array.len() {
282+
return SimplifiedTo(array[index]);
283+
}
284284
}
285+
None
285286
}
286287
Instruction::ArraySet { array, index, value } => {
287288
let array = dfg.get_array_constant(*array);
@@ -290,11 +291,13 @@ impl Instruction {
290291
if let (Some((array, element_type)), Some(index)) = (array, index) {
291292
let index =
292293
index.try_to_u64().expect("Expected array index to fit in u64") as usize;
293-
assert!(index < array.len());
294-
SimplifiedTo(dfg.make_array(array.update(index, *value), element_type))
295-
} else {
296-
None
294+
295+
if index < array.len() {
296+
let new_array = dfg.make_array(array.update(index, *value), element_type);
297+
return SimplifiedTo(new_array);
298+
}
297299
}
300+
None
298301
}
299302
Instruction::Truncate { value, bit_size, .. } => {
300303
if let Some((numeric_constant, typ)) = dfg.get_numeric_constant_with_type(*value) {

crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs

+55-2
Original file line numberDiff line numberDiff line change
@@ -340,14 +340,67 @@ impl<'f> Context<'f> {
340340
self.insert_instruction_with_typevars(enable_side_effects, None);
341341
}
342342

343-
/// Merge two values a and b from separate basic blocks to a single value. This
344-
/// function would return the result of `if c { a } else { b }` as `c*a + (!c)*b`.
343+
/// Merge two values a and b from separate basic blocks to a single value.
344+
/// If these two values are numeric, the result will be
345+
/// `then_condition * then_value + else_condition * else_value`.
346+
/// Otherwise, if the values being merged are arrays, a new array will be made
347+
/// recursively from combining each element of both input arrays.
348+
///
349+
/// It is currently an error to call this function on reference or function values
350+
/// as it is less clear how to merge these.
345351
fn merge_values(
346352
&mut self,
347353
then_condition: ValueId,
348354
else_condition: ValueId,
349355
then_value: ValueId,
350356
else_value: ValueId,
357+
) -> ValueId {
358+
match self.inserter.function.dfg.type_of_value(then_value) {
359+
Type::Numeric(_) => {
360+
self.merge_numeric_values(then_condition, else_condition, then_value, else_value)
361+
}
362+
Type::Array(element_types, len) => {
363+
let mut merged = im::Vector::new();
364+
365+
for i in 0..len {
366+
for (element_index, element_type) in element_types.iter().enumerate() {
367+
let index = ((i * element_types.len() + element_index) as u128).into();
368+
let index = self.inserter.function.dfg.make_constant(index, Type::field());
369+
370+
let typevars = Some(vec![element_type.clone()]);
371+
372+
let mut get_element = |array, typevars| {
373+
let get = Instruction::ArrayGet { array, index };
374+
self.insert_instruction_with_typevars(get, typevars).first()
375+
};
376+
377+
let then_element = get_element(then_value, typevars.clone());
378+
let else_element = get_element(else_value, typevars);
379+
380+
merged.push_back(self.merge_values(
381+
then_condition,
382+
else_condition,
383+
then_element,
384+
else_element,
385+
));
386+
}
387+
}
388+
389+
self.inserter.function.dfg.make_array(merged, element_types)
390+
}
391+
Type::Reference => panic!("Cannot return references from an if expression"),
392+
Type::Function => panic!("Cannot return functions from an if expression"),
393+
}
394+
}
395+
396+
/// Merge two numeric values a and b from separate basic blocks to a single value. This
397+
/// function would return the result of `if c { a } else { b }` as `c*a + (!c)*b`.
398+
fn merge_numeric_values(
399+
&mut self,
400+
then_condition: ValueId,
401+
else_condition: ValueId,
402+
then_value: ValueId,
403+
else_value: ValueId,
351404
) -> ValueId {
352405
let block = self.inserter.function.entry_block();
353406
let mul = Instruction::binary(BinaryOp::Mul, then_condition, then_value);

0 commit comments

Comments
 (0)