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

feat(ssa): Multiple slice mergers #2753

Merged
merged 20 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
44 changes: 36 additions & 8 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,9 +874,13 @@ impl Context {
.expect("ICE: array set on slice must have a length associated with the call");
let length_acir_var = self.convert_value(length, dfg).into_var()?;
let len = self.acir_context.var_to_expression(length_acir_var)?.to_const();
let len = len
.expect("ICE: slice length should be fully tracked and constant by ACIR gen");
len.to_u128() as usize
if let Some(len) = len {
len.to_u128() as usize
} else if let Some((array, _)) = dfg.get_array_constant(array) {
array.len()
} else {
panic!("ICE: slice length should be fully tracked and constant by ACIR gen");
}
}
_ => unreachable!("ICE - expected an array"),
};
Expand Down Expand Up @@ -1013,7 +1017,7 @@ impl Context {
found: array_typ.to_string(),
call_stack: self.acir_context.get_call_stack(),
}
.into())
.into());
}
}
// We do not have to initialize the last elem size value as that is the maximum array size
Expand Down Expand Up @@ -1551,8 +1555,18 @@ impl Context {

let mut new_slice = Vector::new();
self.slice_intrinsic_input(&mut new_slice, slice)?;
// TODO(#2461): make sure that we have handled nested struct inputs
new_slice.insert(index, element);

// We do not return an index out of bounds error directly here
// as the length of the slice is dynamic, and length of `new_slice`
// represents the capacity of the slice, not the actual length.
//
// Constraints should be generated during SSA gen to tell the user
// they are attempting to insert at too large of an index.
// This check prevents a panic inside of the im::Vector insert method.
if index <= new_slice.len() {
// TODO(#2461): make sure that we have handled nested struct inputs
new_slice.insert(index, element);
}

Ok(vec![
AcirValue::Var(new_slice_length, AcirType::field()),
Expand All @@ -1579,8 +1593,22 @@ impl Context {

let mut new_slice = Vector::new();
self.slice_intrinsic_input(&mut new_slice, slice)?;
// TODO(#2461): make sure that we have handled nested struct inputs
let removed_elem = new_slice.remove(index);

// We do not return an index out of bounds error directly here
// as the length of the slice is dynamic, and length of `new_slice`
// represents the capacity of the slice, not the actual length.
//
// Constraints should be generated during SSA gen to tell the user
// they are attempting to remove at too large of an index.
// This check prevents a panic inside of the im::Vector remove method.
let removed_elem = if index < new_slice.len() {
// TODO(#2461): make sure that we have handled nested struct inputs
new_slice.remove(index)
} else {
// This is a dummy value which should never be used if the appropriate
// slice access checks are generated before this slice remove call.
AcirValue::Var(slice_length, AcirType::field())
};

Ok(vec![
AcirValue::Var(new_slice_length, AcirType::field()),
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,10 @@ impl FunctionBuilder {
pub(crate) fn import_intrinsic_id(&mut self, intrinsic: Intrinsic) -> ValueId {
self.current_function.dfg.import_intrinsic(intrinsic)
}

pub(crate) fn get_intrinsic_from_value(&mut self, value: ValueId) -> Option<Intrinsic> {
self.current_function.dfg.get_intrinsic_from_value(value)
}
}

impl std::ops::Index<ValueId> for FunctionBuilder {
Expand Down
26 changes: 26 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,32 @@ impl DataFlowGraph {
self.intrinsics.get(&intrinsic)
}

pub(crate) fn get_intrinsic_from_value(&self, value: ValueId) -> Option<Intrinsic> {
let intrinsics = self
.intrinsics
.iter()
.filter_map(
|(intrinsic, intrinsic_value_id)| {
jfecher marked this conversation as resolved.
Show resolved Hide resolved
if value == *intrinsic_value_id {
Some(*intrinsic)
} else {
None
}
},
)
.collect::<Vec<_>>();
if intrinsics.is_empty() {
None
} else {
assert_eq!(
intrinsics.len(),
1,
"ICE: Multiple intrinsics associated with the same ValueId"
);
Some(intrinsics[0])
}
}

/// Attaches results to the instruction, clearing any previous results.
///
/// This does not normally need to be called manually as it is called within
Expand Down
175 changes: 143 additions & 32 deletions compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ use acvm::{acir::BlackBoxFunc, BlackBoxResolutionError, FieldElement};
use iter_extended::vecmap;
use num_bigint::BigUint;

use crate::ssa::ir::{
basic_block::BasicBlockId,
dfg::DataFlowGraph,
instruction::Intrinsic,
map::Id,
types::Type,
value::{Value, ValueId},
use crate::ssa::{
ir::{
basic_block::BasicBlockId,
dfg::{CallStack, DataFlowGraph},
instruction::Intrinsic,
map::Id,
types::Type,
value::{Value, ValueId},
},
opt::flatten_cfg::value_merger::ValueMerger,
};

use super::{Binary, BinaryOp, Endian, Instruction, SimplifyResult};
Expand Down Expand Up @@ -92,14 +95,22 @@ pub(super) fn simplify_call(
Intrinsic::SlicePushBack => {
let slice = dfg.get_array_constant(arguments[1]);
if let Some((mut slice, element_type)) = slice {
for elem in &arguments[2..] {
slice.push_back(*elem);
// TODO(#2752): We need to handle the element_type size to appropriately handle slices of complex types.
// This is reliant on dynamic indices of non-homogenous slices also being implemented.
if element_type.element_size() != 1 {
// Old code before implementing multiple slice mergers
for elem in &arguments[2..] {
slice.push_back(*elem);
}

let new_slice_length =
update_slice_length(arguments[0], dfg, BinaryOp::Add, block);

let new_slice = dfg.make_array(slice, element_type);
return SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]);
}

let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Add, block);

let new_slice = dfg.make_array(slice, element_type);
SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
simplify_slice_push_back(slice, element_type, arguments, dfg, block)
} else {
SimplifyResult::None
}
Expand All @@ -121,25 +132,8 @@ pub(super) fn simplify_call(
}
Intrinsic::SlicePopBack => {
let slice = dfg.get_array_constant(arguments[1]);
if let Some((mut slice, typ)) = slice {
let element_count = typ.element_size();
let mut results = VecDeque::with_capacity(element_count + 1);

// We must pop multiple elements in the case of a slice of tuples
for _ in 0..element_count {
let elem = slice
.pop_back()
.expect("There are no elements in this slice to be removed");
results.push_front(elem);
}

let new_slice = dfg.make_array(slice, typ);
results.push_front(new_slice);

let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Sub, block);

results.push_front(new_slice_length);
SimplifyResult::SimplifiedToMultiple(results.into())
if let Some((_, typ)) = slice {
simplify_slice_pop_back(typ, arguments, dfg, block)
} else {
SimplifyResult::None
}
Expand Down Expand Up @@ -174,6 +168,14 @@ pub(super) fn simplify_call(
let elements = &arguments[3..];
let mut index = index.to_u128() as usize * elements.len();

// Do not simplify the index is greater than the slice capacity
// or else we will panic inside of the im::Vector insert method
// Constraints should be generated during SSA gen to tell the user
// they are attempting to insert at too large of an index
if index > slice.len() {
return SimplifyResult::None;
}

for elem in &arguments[3..] {
slice.insert(index, *elem);
index += 1;
Expand All @@ -195,6 +197,14 @@ pub(super) fn simplify_call(
let mut results = Vec::with_capacity(element_count + 1);
let index = index.to_u128() as usize * element_count;

// Do not simplify if the index is not less than the slice capacity
// or else we will panic inside of the im::Vector remove method.
// Constraints should be generated during SSA gen to tell the user
// they are attempting to remove at too large of an index.
if index >= slice.len() {
return SimplifyResult::None;
}

for _ in 0..element_count {
results.push(slice.remove(index));
}
Expand Down Expand Up @@ -257,6 +267,107 @@ fn update_slice_length(
dfg.insert_instruction_and_results(instruction, block, None, call_stack).first()
}

fn simplify_slice_push_back(
mut slice: im::Vector<ValueId>,
element_type: Type,
arguments: &[ValueId],
dfg: &mut DataFlowGraph,
block: BasicBlockId,
) -> SimplifyResult {
// The capacity must be an integer so that we can compare it against the slice length which is represented as a field
let capacity = dfg.make_constant((slice.len() as u128).into(), Type::unsigned(64));
let len_equals_capacity_instr =
Instruction::Binary(Binary { lhs: arguments[0], operator: BinaryOp::Eq, rhs: capacity });
let call_stack = dfg.get_value_call_stack(arguments[0]);
let len_equals_capacity = dfg
.insert_instruction_and_results(len_equals_capacity_instr, block, None, call_stack.clone())
.first();
let len_not_equals_capacity_instr = Instruction::Not(len_equals_capacity);
let len_not_equals_capacity = dfg
.insert_instruction_and_results(
len_not_equals_capacity_instr,
block,
None,
call_stack.clone(),
)
.first();

let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Add, block);

for elem in &arguments[2..] {
slice.push_back(*elem);
}
let new_slice = dfg.make_array(slice, element_type);

let set_last_slice_value_instr = Instruction::ArraySet {
array: new_slice,
index: arguments[0],
value: arguments[2],
length: Some(new_slice_length),
};
let set_last_slice_value = dfg
.insert_instruction_and_results(set_last_slice_value_instr, block, None, call_stack)
.first();

let mut value_merger = ValueMerger::new(dfg, block, None, None);
let new_slice = value_merger.merge_values(
len_not_equals_capacity,
len_equals_capacity,
set_last_slice_value,
new_slice,
);

SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
}

fn simplify_slice_pop_back(
element_type: Type,
arguments: &[ValueId],
dfg: &mut DataFlowGraph,
block: BasicBlockId,
) -> SimplifyResult {
let element_types = match element_type.clone() {
Type::Slice(element_types) | Type::Array(element_types, _) => element_types,
_ => {
unreachable!("ICE: Expected slice or array, but got {element_type}");
}
};

let element_count = element_type.element_size();
let mut results = VecDeque::with_capacity(element_count + 1);

let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Sub, block);

let element_size = dfg.make_constant((element_count as u128).into(), Type::field());
let flattened_len_instr = Instruction::binary(BinaryOp::Mul, arguments[0], element_size);
let mut flattened_len = dfg
.insert_instruction_and_results(flattened_len_instr, block, None, CallStack::new())
.first();
flattened_len = update_slice_length(flattened_len, dfg, BinaryOp::Sub, block);

// We must pop multiple elements in the case of a slice of tuples
for _ in 0..element_count {
let get_last_elem_instr =
Instruction::ArrayGet { array: arguments[1], index: flattened_len };
let get_last_elem = dfg
.insert_instruction_and_results(
get_last_elem_instr,
block,
Some(element_types.to_vec()),
CallStack::new(),
)
.first();
results.push_front(get_last_elem);

flattened_len = update_slice_length(flattened_len, dfg, BinaryOp::Sub, block);
}

results.push_front(arguments[1]);

results.push_front(new_slice_length);
SimplifyResult::SimplifiedToMultiple(results.into())
}

/// Try to simplify this black box call. If the call can be simplified to a known value,
/// that value is returned. Otherwise [`SimplifyResult::None`] is returned.
fn simplify_black_box_func(
Expand Down
Loading
Loading