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

chore(ssa): Remove values from array type #771

Merged
merged 8 commits into from
Feb 14, 2023
Merged
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
27 changes: 7 additions & 20 deletions crates/noirc_evaluator/src/ssa/acir_gen/acir_mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,9 @@ impl AcirMem {
// Loads the associated `InternalVar` for the element
// in the `array` at the given `offset`.
//
// First we check if the address of the array element
// is in the memory_map. If not, then we check the `array`
// We check if the address of the array element
// is in the memory_map.
//
// We do not check the `MemArray` initially because the
// `MemoryMap` holds the most updated InternalVar
// associated to the array element.
// TODO: specify what could change between the two?
//
// Returns `None` if `offset` is out of bounds.
pub(crate) fn load_array_element_constant_index(
Expand All @@ -76,19 +72,10 @@ impl AcirMem {
}

// Check the memory_map to see if the element is there
if let Some(internal_var) = self.array_map_mut(array.id).get(&offset) {
return Some(internal_var.clone());
};

let array_element = array.values[offset as usize].clone();

// Compiler sanity check
//
// Since the only time we take the array values
// from the array is when it has been defined in the
// ABI. We know that it must have been initialized with a `Witness`
array_element.cached_witness().expect("ICE: since the value is not in the memory_map it must have came from the ABI, so it is a Witness");

Some(array_element)
let array_element = self
.array_map_mut(array.id)
.get(&offset)
.expect("ICE: Could not find value at index {offset}");
Some(array_element.clone())
}
}
3 changes: 0 additions & 3 deletions crates/noirc_evaluator/src/ssa/acir_gen/internal_var.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ impl InternalVar {
pub(crate) fn set_id(&mut self, id: NodeId) {
self.id = Some(id)
}
pub(crate) fn cached_witness(&self) -> &Option<Witness> {
&self.cached_witness
}

pub fn to_expression(&self) -> Expression {
if let Some(w) = self.cached_witness {
Expand Down
19 changes: 4 additions & 15 deletions crates/noirc_evaluator/src/ssa/conditional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,17 +438,6 @@ impl DecisionTree {
Ok(())
}

//assigns the arrays to the block where they are seen for the first time
fn new_array(ctx: &SsaContext, array_id: super::mem::ArrayId, stack: &mut StackFrame) {
if let std::collections::hash_map::Entry::Vacant(e) = stack.created_arrays.entry(array_id) {
if !ctx.mem[array_id].values.is_empty() {
e.insert(ctx.first_block);
} else {
e.insert(stack.block);
}
}
}

fn short_circuit(
ctx: &mut SsaContext,
stack: &mut StackFrame,
Expand Down Expand Up @@ -511,17 +500,17 @@ impl DecisionTree {
match &ins1.operation {
Operation::Call { returned_arrays, .. } => {
for a in returned_arrays {
DecisionTree::new_array(ctx, a.0, stack);
stack.new_array(a.0);
}
}
Operation::Store { array_id, index, .. } => {
if *index != NodeId::dummy() {
DecisionTree::new_array(ctx, *array_id, stack);
stack.new_array(*array_id);
}
}
_ => {
if let ObjectType::Pointer(a) = ins1.res_type {
DecisionTree::new_array(ctx, a, stack);
stack.new_array(a);
}
}
}
Expand Down Expand Up @@ -718,7 +707,7 @@ impl DecisionTree {
val_false: ctx.one(),
};
if ctx.is_zero(*expr) {
stack.clear();
stack.stack.clear();
}
let cond = ctx.add_instruction(Instruction::new(
operation,
Expand Down
19 changes: 16 additions & 3 deletions crates/noirc_evaluator/src/ssa/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -907,12 +907,25 @@ impl SsaContext {
}

fn init_array(&mut self, array_id: ArrayId, stack_frame: &mut StackFrame) {
let len = self.mem[array_id].len;
let len = self.mem[array_id].len as usize;
let e_type = self.mem[array_id].element_type;
for i in 0..len {
let values = vec![self.zero_with_type(e_type); len];
self.init_array_from_values(array_id, values, stack_frame);
}

pub fn init_array_from_values(
&mut self,
array_id: ArrayId,
values: Vec<NodeId>,
stack_frame: &mut StackFrame,
) {
let len = self.mem[array_id].len as usize;
let e_type = self.mem[array_id].element_type;
assert_eq!(len, values.len());
for (i, v) in values.iter().enumerate() {
let index =
self.get_or_create_const(FieldElement::from(i as i128), ObjectType::Unsigned(32));
let op_a = Operation::Store { array_id, index, value: self.zero_with_type(e_type) };
let op_a = Operation::Store { array_id, index, value: *v };
self.new_instruction_inline(op_a, e_type, stack_frame);
}
}
Expand Down
14 changes: 7 additions & 7 deletions crates/noirc_evaluator/src/ssa/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,6 @@ impl StackFrame {
}
}

pub fn clear(&mut self) {
self.stack.clear();
self.array_map.clear();
self.created_arrays.clear();
self.lca_cache.clear();
}

pub fn push(&mut self, ins_id: NodeId) {
self.stack.push(ins_id);
}
Expand Down Expand Up @@ -179,6 +172,13 @@ impl StackFrame {
true
}
}

//assigns the arrays to the block where they are seen for the first time
pub fn new_array(&mut self, array_id: ArrayId) {
if let std::collections::hash_map::Entry::Vacant(e) = self.created_arrays.entry(array_id) {
e.insert(self.block);
}
}
}

//inline a function call
Expand Down
3 changes: 0 additions & 3 deletions crates/noirc_evaluator/src/ssa/mem.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::ssa::{
acir_gen::InternalVar,
context::SsaContext,
node,
node::{Node, NodeId},
Expand Down Expand Up @@ -30,7 +29,6 @@ impl ArrayId {
pub struct MemArray {
pub id: ArrayId,
pub element_type: node::ObjectType, //type of elements
pub values: Vec<InternalVar>,
pub name: String,
pub def: Definition,
pub len: u32, //number of elements
Expand All @@ -51,7 +49,6 @@ impl MemArray {
id,
element_type: of,
name: name.to_string(),
values: Vec::new(),
def: definition,
len,
adr: 0,
Expand Down
17 changes: 15 additions & 2 deletions crates/noirc_evaluator/src/ssa/ssa_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,21 @@ impl IrGenerator {
) -> NodeId {
let element_type = self.get_object_type_from_abi(el_type);
let (v_id, array_idx) = self.new_array(name, element_type, len as u32, ident_def);
self.context.mem[array_idx].values = vecmap(witness, |w| w.into());
self.context.get_current_block_mut().update_variable(v_id, v_id);
let values = vecmap(witness.iter().enumerate(), |(i, w)| {
let mut var = Variable::new(
element_type,
format!("{name}_{i}"),
None,
self.context.current_block,
);
var.witness = Some(*w);
self.context.add_variable(var, None)
});
let mut stack_frame = crate::ssa::inline::StackFrame::new(self.context.current_block);
self.context.init_array_from_values(array_idx, values, &mut stack_frame);
let block = self.context.get_current_block_mut();
block.instructions.extend_from_slice(&stack_frame.stack);
block.update_variable(v_id, v_id);
v_id
}

Expand Down