Skip to content

Commit

Permalink
Revert "Remove returned_arrays and ArraySetId tracking"
Browse files Browse the repository at this point in the history 
This reverts commit ee0c4e7.
  • Loading branch information
@jfecher
jfecher committed Jan 13, 2023
1 parent 6a06443 commit 1382d81
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 56 deletions.
75 changes: 71 additions & 4 deletions crates/noirc_evaluator/src/ssa/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use super::conditional::{DecisionTree, TreeBuilder};
use super::function::{FuncIndex, SSAFunction};
use super::inline::StackFrame;
use super::mem::{ArrayId, Memory};
use super::node::{BinaryOp, FunctionKind, Instruction, NodeId, NodeObj, ObjectType, Operation};
use super::node::{
ArrayIdSet, BinaryOp, FunctionKind, Instruction, NodeId, NodeObj, ObjectType, Operation,
};
use super::{block, builtin, flatten, inline, integer, node, optim};
use std::collections::{HashMap, HashSet};

Expand Down Expand Up @@ -34,6 +36,9 @@ pub struct SsaContext {
pub functions: HashMap<FuncId, function::SSAFunction>,
pub opcode_ids: HashMap<builtin::Opcode, NodeId>,

/// Maps ArrayIdSet -> Vec<(ArrayId, u32)>,
pub function_returned_arrays: Vec<Vec<(ArrayId, /*result_index:*/ u32)>>,

//Adjacency Matrix of the call graph; list of rows where each row indicates the functions called by the function whose FuncIndex is the row number
pub call_graph: Vec<Vec<u8>>,
dummy_store: HashMap<ArrayId, NodeId>,
Expand All @@ -51,6 +56,7 @@ impl SsaContext {
sealed_blocks: HashSet::new(),
mem: Memory::default(),
functions: HashMap::new(),
function_returned_arrays: Vec::new(),
opcode_ids: HashMap::new(),
call_graph: Vec::new(),
dummy_store: HashMap::new(),
Expand Down Expand Up @@ -1030,7 +1036,8 @@ impl SsaContext {
let block1 = self[exit_block].predecessor[0];
let block2 = self[exit_block].predecessor[1];

let a_type = self.get_object_type(a);
let mut a_type = self.get_object_type(a);
let b_type = self.get_object_type(b);

let name = format!("if_{}_ret{c}", exit_block.0.into_raw_parts().0);
*c += 1;
Expand All @@ -1055,6 +1062,11 @@ impl SsaContext {
}
id
} else {
if let (ObjectType::Function(a), ObjectType::Function(b)) = (a_type, b_type) {
let new_array_set_id = self.union_array_sets(a, b);
a_type = ObjectType::Function(new_array_set_id);
}

let new_var = node::Variable::new(a_type, name, None, exit_block);
let v = self.add_variable(new_var, None);
let operation = Operation::Phi { root: v, block_args: vec![(a, block1), (b, block2)] };
Expand All @@ -1074,6 +1086,10 @@ impl SsaContext {
NodeId(index)
}

pub fn set_function_type(&mut self, func_id: FuncId, node_id: NodeId, typ: ObjectType) {
self[node_id] = NodeObj::Function(FunctionKind::Normal(func_id), node_id, typ);
}

/// Return the standard NodeId for this FuncId.
/// The 'standard' NodeId is just the NodeId assigned to the function when it
/// is first compiled so that repeated NodeObjs are not made for the same function.
Expand All @@ -1091,8 +1107,18 @@ impl SsaContext {
return *id;
}

let (len, elem_type) = opcode.get_result_type();
let array_set = if len > 1 {
let array = self.new_array(&format!("{}_result", opcode), elem_type, len, None).1;
self.push_array_set(vec![(array, 0)])
} else {
self.push_array_set(vec![])
};

let index = self.nodes.insert_with(|index| {
NodeObj::Function(FunctionKind::Builtin(opcode), NodeId(index), ObjectType::Function)
let node_id = NodeId(index);
let typ = ObjectType::Function(array_set);
NodeObj::Function(FunctionKind::Builtin(opcode), node_id, typ)
});
self.opcode_ids.insert(opcode, NodeId(index));
NodeId(index)
Expand All @@ -1105,6 +1131,33 @@ impl SsaContext {
}
}

pub fn push_array_set(&mut self, arrays: Vec<(ArrayId, u32)>) -> ArrayIdSet {
let next_id = self.function_returned_arrays.len();
self.function_returned_arrays.push(arrays);
ArrayIdSet(next_id as u32)
}

pub fn union_array_sets(&mut self, first: ArrayIdSet, second: ArrayIdSet) -> ArrayIdSet {
let mut union_set = self.function_returned_arrays[first.0 as usize].clone();
union_set.extend_from_slice(&self.function_returned_arrays[second.0 as usize]);
// The sort + dedup here is expected to be cheap as the length of this set matches the number
// of array parameters of both functions, and the average function only uses roughly 0-2 array parameters.
// `union_array_sets` as a whole is also called fairly infrequently - only when function
// values are unified in a Phi instruction, e.g. from being returned by an if expression.
union_set.sort();
union_set.dedup();
self.push_array_set(union_set)
}

pub fn get_returned_arrays(&self, func: NodeId) -> Vec<(ArrayId, u32)> {
match self[func].get_type() {
ObjectType::Function(arrays) => {
self.function_returned_arrays[arrays.0 as usize].clone()
}
other => unreachable!("get_returned_arrays: Expected function type, found {:?}", other),
}
}

pub fn convert_type(&mut self, t: &noirc_frontend::monomorphisation::ast::Type) -> ObjectType {
use noirc_frontend::monomorphisation::ast::Type;
use noirc_frontend::Signedness;
Expand All @@ -1123,7 +1176,13 @@ impl SsaContext {
}
Type::Array(..) => panic!("Cannot convert an array type {} into an ObjectType since it is unknown which array it refers to", t),
Type::Unit => ObjectType::NotAnObject,
Type::Function(..) => ObjectType::Function,
Type::Function(..) => {
// TODO #612: We should not track arrays through ObjectTypes.
// This is incorrect, we cannot track arrays through function types in this case
// since we do not know which arrays the function uses at this point.
let id = self.push_array_set(vec![]);
ObjectType::Function(id)
}
Type::Tuple(_) => todo!("Conversion to ObjectType is unimplemented for tuples"),
}
}
Expand Down Expand Up @@ -1207,3 +1266,11 @@ impl std::ops::IndexMut<NodeId> for SsaContext {
&mut self.nodes[index.0]
}
}

impl std::ops::Index<ArrayIdSet> for SsaContext {
type Output = Vec<(ArrayId, u32)>;

fn index(&self, index: ArrayIdSet) -> &Self::Output {
&self.function_returned_arrays[index.0 as usize]
}
}
100 changes: 50 additions & 50 deletions crates/noirc_evaluator/src/ssa/function.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use noirc_frontend::monomorphisation::ast::{Call, Definition, FuncId, LocalId, T

use super::builtin;
use super::conditional::{AssumptionId, DecisionTree, TreeBuilder};
use super::node::{Node, Operation};
use super::node::{ArrayIdSet, Node, Operation};
use super::{
block,
block::BlockId,
Expand Down Expand Up @@ -48,10 +48,12 @@ impl SSAFunction {
idx: FuncIndex,
ctx: &mut SsaContext,
) -> SSAFunction {
// Temporary ArrayIdSet - the actual arrays returned by this function are not yet known.
let typ = ObjectType::Function(ArrayIdSet(u32::MAX));
SSAFunction {
entry_block: block_id,
id,
node_id: ctx.push_function_id(id, ObjectType::Function),
node_id: ctx.push_function_id(id, typ),
name: name.to_string(),
arguments: Vec::new(),
result_types: Vec::new(),
Expand Down Expand Up @@ -179,32 +181,46 @@ impl IRGenerator {
let function_body = self.program.take_function_body(func_id);
let last_value = self.codegen_expression(&function_body)?;
let return_values = last_value.to_node_ids();
let mut returned_arrays = vec![];

func.result_types.clear();
let return_values = try_vecmap(return_values, |mut return_id| {
let node_opt = self.context.try_get_node(return_id);
let typ = node_opt.map_or(ObjectType::NotAnObject, |node| node.get_type());

if let Some(ins) = self.context.try_get_instruction(return_id) {
if ins.operation.opcode() == Opcode::Results {
// n.b. this required for result instructions, but won't hurt if done for all i
let new_var = node::Variable {
id: NodeId::dummy(),
obj_type: typ,
name: format!("return_{}", return_id.0.into_raw_parts().0),
root: None,
def: None,
witness: None,
parent_block: self.context.current_block,
};
let b_id = self.context.add_variable(new_var, None);
let b_id1 = self.context.handle_assign(b_id, None, return_id)?;
return_id = ssa_form::get_current_value(&mut self.context, b_id1);
let return_values =
try_vecmap(return_values.into_iter().enumerate(), |(i, mut return_id)| {
let node_opt = self.context.try_get_node(return_id);
let typ = node_opt.map_or(ObjectType::NotAnObject, |node| {
let typ = node.get_type();
if let ObjectType::Pointer(array_id) = typ {
returned_arrays.push((array_id, i as u32));
}
typ
});

if let Some(ins) = self.context.try_get_instruction(return_id) {
if ins.operation.opcode() == Opcode::Results {
// n.b. this required for result instructions, but won't hurt if done for all i
let new_var = node::Variable {
id: NodeId::dummy(),
obj_type: typ,
name: format!("return_{}", return_id.0.into_raw_parts().0),
root: None,
def: None,
witness: None,
parent_block: self.context.current_block,
};
let b_id = self.context.add_variable(new_var, None);
let b_id1 = self.context.handle_assign(b_id, None, return_id)?;
return_id = ssa_form::get_current_value(&mut self.context, b_id1);
}
}
}
func.result_types.push(typ);
Ok::<NodeId, RuntimeError>(return_id)
})?;
func.result_types.push(typ);
Ok::<NodeId, RuntimeError>(return_id)
})?;

let array_set_id = self.context.push_array_set(returned_arrays);
let function_type = ObjectType::Function(array_set_id);

// Push a standard NodeId for this FuncId
self.context.set_function_type(func_id, func.node_id, function_type);

self.context.new_instruction(
node::Operation::Return(return_values),
Expand All @@ -216,7 +232,7 @@ impl IRGenerator {
self.context.current_block = current_block;
self.function_context = current_function;

Ok(ObjectType::Function)
Ok(function_type)
}

fn create_function_parameter(&mut self, id: LocalId, typ: &Type, name: &str) -> Vec<NodeId> {
Expand All @@ -238,46 +254,30 @@ impl IRGenerator {
self.call_low_level(opcode, arguments)
} else {
let return_types = call.return_type.flatten().into_iter().enumerate();
let returned_arrays = self.context.get_returned_arrays(func);
let predicate = AssumptionId::dummy();
let location = call.location;

let call = Operation::Call {
func,
arguments: arguments.clone(),
returned_arrays: vec![],
arguments,
returned_arrays: returned_arrays.clone(),
predicate,
location,
};

let call_instruction = self.context.new_instruction(call, ObjectType::NotAnObject)?;
let mut returned_arrays = vec![];

let result_ids = try_vecmap(return_types, |(i, typ)| {
try_vecmap(return_types, |(i, typ)| {
let result = Operation::Result { call_instruction, index: i as u32 };
let typ = match typ {
Type::Array(_, elem_type) => {
let elem_type = self.context.convert_type(&elem_type);

// FIXME: We crash in anchor.rs if the user tries to use an index
// above this temporary length.
let max_array_size = 100_000;

let array_id =
self.context.new_array("", elem_type, max_array_size, None).1;
returned_arrays.push((array_id, i as u32));
ObjectType::Pointer(array_id)
Type::Array(_, _) => {
let id = returned_arrays.iter().find(|(_, index)| *index == i as u32);
ObjectType::Pointer(id.unwrap().0)
}
other => self.context.convert_type(&other),
};

self.context.new_instruction(result, typ)
});

// Update the call instruction with the fresh array ids in returned_arrays
self.context.get_mut_instruction(call_instruction).operation =
Operation::Call { func, arguments, returned_arrays, predicate, location };

result_ids
})
}
}

Expand Down
7 changes: 5 additions & 2 deletions crates/noirc_evaluator/src/ssa/node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,15 @@ pub enum ObjectType {
Signed(u32), //bit size
Pointer(ArrayId),

Function,
Function(ArrayIdSet), // Function with a set of arrays that may be returned
//TODO big_int
//TODO floats
NotAnObject, //not an object
}

#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct ArrayIdSet(pub u32);

#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum NumericType {
Signed(u32),
Expand Down Expand Up @@ -224,7 +227,7 @@ impl ObjectType {
ObjectType::Signed(c) => *c,
ObjectType::Unsigned(c) => *c,
ObjectType::Pointer(_) => 0,
ObjectType::Function => 0,
ObjectType::Function(_) => 0,
}
}

Expand Down

0 comments on commit 1382d81

Please sign in to comment.