From f3e30a11917c5b5110bbf669b5e2ee10e6cafb43 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 12 Dec 2024 13:29:41 +0000 Subject: [PATCH 01/13] manage call stacks with a tree --- compiler/noirc_evaluator/src/acir/mod.rs | 5 +- .../src/brillig/brillig_gen/brillig_block.rs | 2 +- .../check_for_underconstrained_values.rs | 2 +- .../src/ssa/function_builder/mod.rs | 24 +-- .../noirc_evaluator/src/ssa/ir/basic_block.rs | 4 +- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 157 +++++++++++++++++- .../noirc_evaluator/src/ssa/ir/function.rs | 3 + .../src/ssa/ir/function_inserter.rs | 9 +- .../noirc_evaluator/src/ssa/ir/instruction.rs | 31 ++-- .../src/ssa/ir/instruction/call.rs | 56 +++---- .../src/ssa/ir/instruction/call/blackbox.rs | 28 ++-- .../src/ssa/opt/assert_constant.rs | 6 +- .../src/ssa/opt/constant_folding.rs | 4 +- compiler/noirc_evaluator/src/ssa/opt/die.rs | 18 +- .../src/ssa/opt/flatten_cfg.rs | 87 ++++------ .../src/ssa/opt/flatten_cfg/value_merger.rs | 62 +++---- .../noirc_evaluator/src/ssa/opt/inlining.rs | 66 ++++++-- .../src/ssa/opt/normalize_value_ids.rs | 9 +- .../src/ssa/opt/remove_bit_shifts.rs | 11 +- .../src/ssa/opt/remove_if_else.rs | 2 +- .../src/ssa/opt/simplify_cfg.rs | 4 +- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 18 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 6 +- 23 files changed, 372 insertions(+), 242 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 76f0dea95bb..2bce4e8ea4b 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -674,7 +674,7 @@ impl<'a> Context<'a> { brillig: &Brillig, ) -> Result, RuntimeError> { let instruction = &dfg[instruction_id]; - self.acir_context.set_call_stack(dfg.get_call_stack(instruction_id)); + self.acir_context.set_call_stack(dfg.get_instruction_call_stack(instruction_id)); let mut warnings = Vec::new(); match instruction { Instruction::Binary(binary) => { @@ -1823,7 +1823,7 @@ impl<'a> Context<'a> { ) -> Result<(Vec, Vec), RuntimeError> { let (return_values, call_stack) = match terminator { TerminatorInstruction::Return { return_values, call_stack } => { - (return_values, call_stack.clone()) + (return_values, *call_stack) } // TODO(https://github.com/noir-lang/noir/issues/4616): Enable recursion on foldable/non-inlined ACIR functions _ => unreachable!("ICE: Program must have a singular return"), @@ -1842,6 +1842,7 @@ impl<'a> Context<'a> { } } + let call_stack = dfg.get_call_stack(call_stack); let warnings = if has_constant_return { vec![SsaReport::Warning(InternalWarning::ReturnConstant { call_stack })] } else { diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index 9c88c559b59..bb6d93caae1 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -201,7 +201,7 @@ impl<'block> BrilligBlock<'block> { /// Converts an SSA instruction into a sequence of Brillig opcodes. fn convert_ssa_instruction(&mut self, instruction_id: InstructionId, dfg: &DataFlowGraph) { let instruction = &dfg[instruction_id]; - self.brillig_context.set_call_stack(dfg.get_call_stack(instruction_id)); + self.brillig_context.set_call_stack(dfg.get_instruction_call_stack(instruction_id)); self.initialize_constants( &self.function_context.constant_allocation.allocated_at_location( diff --git a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 7a4e336c33e..6233a6165f4 100644 --- a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -145,7 +145,7 @@ impl Context { // There is a value not in the set, which means that the inputs/outputs of this call have not been properly constrained if unused_inputs { warnings.push(SsaReport::Bug(InternalBug::IndependentSubgraph { - call_stack: function.dfg.get_call_stack( + call_stack: function.dfg.get_instruction_call_stack( self.brillig_return_to_instruction_id[&brillig_output_in_set], ), })); diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 0ae61404442..0aaefffa414 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -18,7 +18,7 @@ use crate::ssa::ir::{ use super::{ ir::{ basic_block::BasicBlock, - dfg::{CallStack, InsertInstructionResult}, + dfg::{CallStack, CallStackId, InsertInstructionResult}, function::RuntimeType, instruction::{ConstrainError, InstructionId, Intrinsic}, }, @@ -36,7 +36,7 @@ pub(crate) struct FunctionBuilder { pub(super) current_function: Function, current_block: BasicBlockId, finished_functions: Vec, - call_stack: CallStack, + call_stack: CallStackId, error_types: BTreeMap, } @@ -52,7 +52,7 @@ impl FunctionBuilder { current_block: new_function.entry_block(), current_function: new_function, finished_functions: Vec::new(), - call_stack: CallStack::new(), + call_stack: CallStackId::root(), error_types: BTreeMap::default(), } } @@ -77,11 +77,13 @@ impl FunctionBuilder { function_id: FunctionId, runtime_type: RuntimeType, ) { + let call_stack = self.current_function.dfg.get_call_stack(self.call_stack); let mut new_function = Function::new(name, function_id); new_function.set_runtime(runtime_type); self.current_block = new_function.entry_block(); - let old_function = std::mem::replace(&mut self.current_function, new_function); + // Copy the call stack to the new function + self.call_stack = self.current_function.dfg.get_or_insert_locations(call_stack); self.finished_functions.push(old_function); } @@ -170,7 +172,7 @@ impl FunctionBuilder { instruction, block, ctrl_typevars, - self.call_stack.clone(), + self.call_stack, ) } @@ -195,17 +197,17 @@ impl FunctionBuilder { } pub(crate) fn set_location(&mut self, location: Location) -> &mut FunctionBuilder { - self.call_stack = im::Vector::unit(location); + self.call_stack = self.current_function.dfg.add_location_to_root(location); self } - pub(crate) fn set_call_stack(&mut self, call_stack: CallStack) -> &mut FunctionBuilder { + pub(crate) fn set_call_stack(&mut self, call_stack: CallStackId) -> &mut FunctionBuilder { self.call_stack = call_stack; self } pub(crate) fn get_call_stack(&self) -> CallStack { - self.call_stack.clone() + self.current_function.dfg.get_call_stack(self.call_stack) } /// Insert a Load instruction at the end of the current block, loading from the given address @@ -377,7 +379,7 @@ impl FunctionBuilder { destination: BasicBlockId, arguments: Vec, ) { - let call_stack = self.call_stack.clone(); + let call_stack = self.call_stack; self.terminate_block_with(TerminatorInstruction::Jmp { destination, arguments, @@ -393,7 +395,7 @@ impl FunctionBuilder { then_destination: BasicBlockId, else_destination: BasicBlockId, ) { - let call_stack = self.call_stack.clone(); + let call_stack = self.call_stack; self.terminate_block_with(TerminatorInstruction::JmpIf { condition, then_destination, @@ -404,7 +406,7 @@ impl FunctionBuilder { /// Terminate the current block with a return instruction pub(crate) fn terminate_with_return(&mut self, return_values: Vec) { - let call_stack = self.call_stack.clone(); + let call_stack = self.call_stack; self.terminate_block_with(TerminatorInstruction::Return { return_values, call_stack }); } diff --git a/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs b/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs index a7c637dedd0..069e1f49164 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs @@ -1,5 +1,5 @@ use super::{ - dfg::CallStack, + dfg::CallStackId, instruction::{InstructionId, TerminatorInstruction}, map::Id, value::ValueId, @@ -123,7 +123,7 @@ impl BasicBlock { terminator, TerminatorInstruction::Return { return_values: Vec::new(), - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }, ) } diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 827944e22d1..5cd1b4c1fe8 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -21,6 +21,75 @@ use serde::{Deserialize, Serialize}; use serde_with::serde_as; use serde_with::DisplayFromStr; +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub(crate) struct CallStackId(u32); + +impl CallStackId { + pub(crate) fn root() -> Self { + Self::new(0) + } + + fn new(id: usize) -> Self { + Self(id as u32) + } + + pub(crate) fn index(&self) -> usize { + self.0 as usize + } + + pub(crate) fn is_root(&self) -> bool { + self.0 == 0 + } + + // Retrieves a CallStack from a CallStackId + fn get_call_stack(&self, locations: &[LocationNode]) -> CallStack { + let mut call_stack = im::Vector::new(); + let mut current_location = *self; + while let Some(parent) = locations[current_location.index()].parent { + call_stack.push_front(locations[current_location.index()].value); + current_location = parent; + } + call_stack + } + + // Adds a location to the call stack + fn add_location(&self, location: Location, locations: &mut Vec) -> CallStackId { + if let Some(result) = locations[self.index()] + .children + .iter() + .find(|child| locations[child.index()].value == location) + { + return *result; + } + locations.push(LocationNode { parent: Some(*self), children: vec![], value: location }); + let new_location = CallStackId::new(locations.len() - 1); + locations[self.index()].children.push(new_location); + new_location + } + + // Returns a new CallStackId which extends the current one with the provided call_stack. + fn extend(&self, call_stack: &CallStack, locations: &mut Vec) -> CallStackId { + let mut result = *self; + for location in call_stack { + result = result.add_location(*location, locations); + } + result + } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct LocationNode { + parent: Option, + children: Vec, + value: Location, +} + +impl LocationNode { + fn new() -> Self { + Self { parent: None, children: vec![], value: Location::dummy() } + } +} + /// The DataFlowGraph contains most of the actual data in a function including /// its blocks, instructions, and values. This struct is largely responsible for /// owning most data in a function and handing out Ids to this data that can be @@ -91,7 +160,9 @@ pub(crate) struct DataFlowGraph { /// Instructions inserted by internal SSA passes that don't correspond to user code /// may not have a corresponding location. #[serde(skip)] - locations: HashMap, + locations: HashMap, + + location_array: Vec, #[serde(skip)] pub(crate) data_bus: DataBus, @@ -170,9 +241,9 @@ impl DataFlowGraph { instruction: Instruction, block: BasicBlockId, ctrl_typevars: Option>, - call_stack: CallStack, + call_stack: CallStackId, ) -> InsertInstructionResult { - match instruction.simplify(self, block, ctrl_typevars.clone(), &call_stack) { + match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) { SimplifyResult::SimplifiedTo(simplification) => { InsertInstructionResult::SimplifiedTo(simplification) } @@ -200,7 +271,7 @@ impl DataFlowGraph { for instruction in instructions { let id = self.make_instruction(instruction, ctrl_typevars.clone()); self.blocks[block].insert_instruction(id); - self.locations.insert(id, call_stack.clone()); + self.locations.insert(id, call_stack); last_id = Some(id); } @@ -485,21 +556,91 @@ impl DataFlowGraph { destination.set_terminator(terminator); } - pub(crate) fn get_call_stack(&self, instruction: InstructionId) -> CallStack { + pub(crate) fn get_instruction_call_stack(&self, instruction: InstructionId) -> CallStack { + let call_stack = self.locations.get(&instruction).cloned().unwrap_or_default(); + self.get_call_stack(call_stack) + } + + pub(crate) fn get_instruction_call_stack_id(&self, instruction: InstructionId) -> CallStackId { self.locations.get(&instruction).cloned().unwrap_or_default() } - pub(crate) fn add_location(&mut self, instruction: InstructionId, location: Location) { - self.locations.entry(instruction).or_default().push_back(location); + pub(crate) fn add_location_to_instruction( + &mut self, + instruction: InstructionId, + location: Location, + ) { + let call_stack = self.locations.entry(instruction).or_default(); + call_stack.add_location(location, &mut self.location_array); + } + + pub(crate) fn add_location_to_root(&mut self, location: Location) -> CallStackId { + if self.location_array.is_empty() { + self.location_array.push(LocationNode { + parent: None, + children: vec![], + value: location, + }); + CallStackId::root() + } else { + CallStackId::root().add_location(location, &mut self.location_array) + } + } + + // Get (or create) a CallStackId corresponding to the given locations + pub(crate) fn get_or_insert_locations(&mut self, locations: CallStack) -> CallStackId { + let mut result = CallStackId::root(); + for location in locations { + result = result.add_location(location, &mut self.location_array); + } + result } pub(crate) fn get_value_call_stack(&self, value: ValueId) -> CallStack { match &self.values[self.resolve(value)] { - Value::Instruction { instruction, .. } => self.get_call_stack(*instruction), + Value::Instruction { instruction, .. } => self.get_instruction_call_stack(*instruction), _ => im::Vector::new(), } } + pub(crate) fn get_value_call_stack_id(&self, value: ValueId) -> CallStackId { + match &self.values[self.resolve(value)] { + Value::Instruction { instruction, .. } => { + self.get_instruction_call_stack_id(*instruction) + } + _ => CallStackId::root(), + } + } + + pub(crate) fn get_call_stack(&self, call_stack: CallStackId) -> CallStack { + call_stack.get_call_stack(&self.location_array) + } + + pub(crate) fn extend_call_stack( + &mut self, + call_stack: CallStackId, + locations: &CallStack, + ) -> CallStackId { + call_stack.extend(locations, &mut self.location_array) + } + + // Retrieve the CallStackId corresponding to call_stack with the last 'len' locations removed. + pub(crate) fn unwind_call_stack( + &self, + mut call_stack: CallStackId, + mut len: usize, + ) -> CallStackId { + while len > 0 { + if let Some(parent) = self.location_array[call_stack.index()].parent { + len -= 1; + call_stack = parent; + } else { + break; + } + } + call_stack + } + /// True if the given ValueId refers to a (recursively) constant value pub(crate) fn is_constant(&self, argument: ValueId) -> bool { match &self[self.resolve(argument)] { diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 6413107c04a..189331a5ac2 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -10,6 +10,7 @@ use super::instruction::TerminatorInstruction; use super::map::Id; use super::types::Type; use super::value::ValueId; +use noirc_errors::Location; #[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, Serialize, Deserialize)] pub(crate) enum RuntimeType { @@ -85,6 +86,8 @@ impl Function { /// Note that any parameters or attributes of the function must be manually added later. pub(crate) fn new(name: String, id: FunctionId) -> Self { let mut dfg = DataFlowGraph::default(); + // Adds root node for the location tree + dfg.add_location_to_root(Location::dummy()); let entry_block = dfg.make_block(); Self { name, id, entry_block, dfg, runtime: RuntimeType::Acir(InlineType::default()) } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs b/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs index 6ebd2aa1105..797feb6a9f5 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs @@ -4,11 +4,12 @@ use crate::ssa::ir::types::Type; use super::{ basic_block::BasicBlockId, - dfg::{CallStack, InsertInstructionResult}, + dfg::InsertInstructionResult, function::Function, instruction::{Instruction, InstructionId}, value::ValueId, }; +use crate::ssa::ir::dfg::CallStackId; use fxhash::FxHashMap as HashMap; /// The FunctionInserter can be used to help modify existing Functions @@ -72,10 +73,10 @@ impl<'f> FunctionInserter<'f> { } /// Get an instruction and make sure all the values in it are freshly resolved. - pub(crate) fn map_instruction(&mut self, id: InstructionId) -> (Instruction, CallStack) { + pub(crate) fn map_instruction(&mut self, id: InstructionId) -> (Instruction, CallStackId) { ( self.function.dfg[id].clone().map_values(|id| self.resolve(id)), - self.function.dfg.get_call_stack(id), + self.function.dfg.get_instruction_call_stack_id(id), ) } @@ -115,7 +116,7 @@ impl<'f> FunctionInserter<'f> { instruction: Instruction, id: InstructionId, mut block: BasicBlockId, - call_stack: CallStack, + call_stack: CallStackId, ) -> InsertInstructionResult { let results = self.function.dfg.instruction_results(id); let results = vecmap(results, |id| self.function.dfg.resolve(*id)); diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 76409f6a20a..a5bc57605be 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -1,4 +1,3 @@ -use noirc_errors::Location; use serde::{Deserialize, Serialize}; use std::hash::{Hash, Hasher}; @@ -15,7 +14,7 @@ use crate::ssa::{ir::function::RuntimeType, opt::flatten_cfg::value_merger::Valu use super::{ basic_block::BasicBlockId, - dfg::{CallStack, DataFlowGraph}, + dfg::{CallStackId, DataFlowGraph}, function::Function, map::Id, types::{NumericType, Type}, @@ -730,7 +729,7 @@ impl Instruction { dfg: &mut DataFlowGraph, block: BasicBlockId, ctrl_typevars: Option>, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { use SimplifyResult::*; match self { @@ -787,7 +786,7 @@ impl Instruction { instruction, block, Option::None, - call_stack.clone(), + call_stack, ); return SimplifiedTo(new_array.first()); } @@ -1132,7 +1131,7 @@ pub(crate) enum TerminatorInstruction { condition: ValueId, then_destination: BasicBlockId, else_destination: BasicBlockId, - call_stack: CallStack, + call_stack: CallStackId, }, /// Unconditional Jump @@ -1140,7 +1139,7 @@ pub(crate) enum TerminatorInstruction { /// Jumps to specified `destination` with `arguments`. /// The CallStack here is expected to be used to issue an error when the start range of /// a for loop cannot be deduced at compile-time. - Jmp { destination: BasicBlockId, arguments: Vec, call_stack: CallStack }, + Jmp { destination: BasicBlockId, arguments: Vec, call_stack: CallStackId }, /// Return from the current function with the given return values. /// @@ -1149,7 +1148,7 @@ pub(crate) enum TerminatorInstruction { /// unconditionally jump to a single exit block with the return values /// as the block arguments. Then the exit block can terminate in a return /// instruction returning these values. - Return { return_values: Vec, call_stack: CallStack }, + Return { return_values: Vec, call_stack: CallStackId }, } impl TerminatorInstruction { @@ -1164,16 +1163,16 @@ impl TerminatorInstruction { condition: f(*condition), then_destination: *then_destination, else_destination: *else_destination, - call_stack: call_stack.clone(), + call_stack: *call_stack, }, Jmp { destination, arguments, call_stack } => Jmp { destination: *destination, arguments: vecmap(arguments, |value| f(*value)), - call_stack: call_stack.clone(), + call_stack: *call_stack, }, Return { return_values, call_stack } => Return { return_values: vecmap(return_values, |value| f(*value)), - call_stack: call_stack.clone(), + call_stack: *call_stack, }, } } @@ -1233,11 +1232,19 @@ impl TerminatorInstruction { } } - pub(crate) fn call_stack(&self) -> im::Vector { + pub(crate) fn call_stack(&self) -> CallStackId { match self { TerminatorInstruction::JmpIf { call_stack, .. } | TerminatorInstruction::Jmp { call_stack, .. } - | TerminatorInstruction::Return { call_stack, .. } => call_stack.clone(), + | TerminatorInstruction::Return { call_stack, .. } => *call_stack, + } + } + + pub(crate) fn set_call_stack(&mut self, new_call_stack: CallStackId) { + match self { + TerminatorInstruction::JmpIf { call_stack, .. } + | TerminatorInstruction::Jmp { call_stack, .. } + | TerminatorInstruction::Return { call_stack, .. } => *call_stack = new_call_stack, } } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index a8db5e2ff94..b2ab97dfa08 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -12,7 +12,7 @@ use num_bigint::BigUint; use crate::ssa::{ ir::{ basic_block::BasicBlockId, - dfg::{CallStack, DataFlowGraph}, + dfg::{CallStackId, DataFlowGraph}, instruction::Intrinsic, map::Id, types::Type, @@ -38,7 +38,7 @@ pub(super) fn simplify_call( dfg: &mut DataFlowGraph, block: BasicBlockId, ctrl_typevars: Option>, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { let intrinsic = match &dfg[func] { Value::Intrinsic(intrinsic) => *intrinsic, @@ -142,14 +142,7 @@ pub(super) fn simplify_call( return SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice]); } - simplify_slice_push_back( - slice, - element_type, - arguments, - dfg, - block, - call_stack.clone(), - ) + simplify_slice_push_back(slice, element_type, arguments, dfg, block, call_stack) } else { SimplifyResult::None } @@ -179,7 +172,7 @@ pub(super) fn simplify_call( let slice = dfg.get_array_constant(arguments[1]); if let Some((_, typ)) = slice { - simplify_slice_pop_back(typ, arguments, dfg, block, call_stack.clone()) + simplify_slice_pop_back(typ, arguments, dfg, block, call_stack) } else { SimplifyResult::None } @@ -350,7 +343,7 @@ pub(super) fn simplify_call( truncate, block, Some(vec![incoming_type]), - call_stack.clone(), + call_stack, ) .first(); @@ -408,7 +401,7 @@ fn update_slice_length( ) -> ValueId { let one = dfg.make_constant(FieldElement::one(), Type::length_type()); let instruction = Instruction::Binary(Binary { lhs: slice_len, operator, rhs: one }); - let call_stack = dfg.get_value_call_stack(slice_len); + let call_stack = dfg.get_value_call_stack_id(slice_len); dfg.insert_instruction_and_results(instruction, block, None, call_stack).first() } @@ -418,23 +411,18 @@ fn simplify_slice_push_back( arguments: &[ValueId], dfg: &mut DataFlowGraph, block: BasicBlockId, - call_stack: CallStack, + call_stack: CallStackId, ) -> SimplifyResult { // The capacity must be an integer so that we can compare it against the slice length let capacity = dfg.make_constant((slice.len() as u128).into(), Type::length_type()); let len_equals_capacity_instr = Instruction::Binary(Binary { lhs: arguments[0], operator: BinaryOp::Eq, rhs: capacity }); let len_equals_capacity = dfg - .insert_instruction_and_results(len_equals_capacity_instr, block, None, call_stack.clone()) + .insert_instruction_and_results(len_equals_capacity_instr, block, None, call_stack) .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(), - ) + .insert_instruction_and_results(len_not_equals_capacity_instr, block, None, call_stack) .first(); let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Add, block); @@ -444,7 +432,7 @@ fn simplify_slice_push_back( } let slice_size = slice.len() as u32; let element_size = element_type.element_size() as u32; - let new_slice = make_array(dfg, slice, element_type, block, &call_stack); + let new_slice = make_array(dfg, slice, element_type, block, call_stack); let set_last_slice_value_instr = Instruction::ArraySet { array: new_slice, @@ -454,7 +442,7 @@ fn simplify_slice_push_back( }; let set_last_slice_value = dfg - .insert_instruction_and_results(set_last_slice_value_instr, block, None, call_stack.clone()) + .insert_instruction_and_results(set_last_slice_value_instr, block, None, call_stack) .first(); let mut slice_sizes = HashMap::default(); @@ -480,7 +468,7 @@ fn simplify_slice_pop_back( arguments: &[ValueId], dfg: &mut DataFlowGraph, block: BasicBlockId, - call_stack: CallStack, + call_stack: CallStackId, ) -> SimplifyResult { let element_types = match element_type.clone() { Type::Slice(element_types) | Type::Array(element_types, _) => element_types, @@ -496,9 +484,8 @@ fn simplify_slice_pop_back( let element_size = dfg.make_constant((element_count as u128).into(), Type::length_type()); 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, call_stack.clone()) - .first(); + let mut flattened_len = + dfg.insert_instruction_and_results(flattened_len_instr, block, None, call_stack).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 @@ -510,7 +497,7 @@ fn simplify_slice_pop_back( get_last_elem_instr, block, Some(element_types.to_vec()), - call_stack.clone(), + call_stack, ) .first(); results.push_front(get_last_elem); @@ -531,7 +518,7 @@ fn simplify_black_box_func( arguments: &[ValueId], dfg: &mut DataFlowGraph, block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { cfg_if::cfg_if! { if #[cfg(feature = "bn254")] { @@ -630,7 +617,7 @@ fn make_constant_array( results: impl Iterator, typ: Type, block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> ValueId { let result_constants: im::Vector<_> = results.map(|element| dfg.make_constant(element, typ.clone())).collect(); @@ -644,10 +631,9 @@ fn make_array( elements: im::Vector, typ: Type, block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> ValueId { let instruction = Instruction::MakeArray { elements, typ }; - let call_stack = call_stack.clone(); dfg.insert_instruction_and_results(instruction, block, None, call_stack).first() } @@ -656,7 +642,7 @@ fn make_constant_slice( results: Vec, typ: Type, block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> (ValueId, ValueId) { let result_constants = vecmap(results, |element| dfg.make_constant(element, typ.clone())); @@ -721,7 +707,7 @@ fn simplify_hash( arguments: &[ValueId], hash_function: fn(&[u8]) -> Result<[u8; 32], BlackBoxResolutionError>, block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { match dfg.get_array_constant(arguments[0]) { Some((input, _)) if array_is_constant(dfg, &input) => { @@ -793,7 +779,7 @@ fn simplify_derive_generators( arguments: &[ValueId], num_generators: u32, block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { if arguments.len() == 2 { let domain_separator_string = dfg.get_array_constant(arguments[0]); diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index 016d7ffa25b..8da63abb11c 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -2,10 +2,11 @@ use std::sync::Arc; use acvm::{acir::AcirField, BlackBoxFunctionSolver, BlackBoxResolutionError, FieldElement}; +use crate::ssa::ir::dfg::CallStackId; use crate::ssa::ir::instruction::BlackBoxFunc; use crate::ssa::ir::{ basic_block::BasicBlockId, - dfg::{CallStack, DataFlowGraph}, + dfg::DataFlowGraph, instruction::{Instruction, Intrinsic, SimplifyResult}, types::Type, value::ValueId, @@ -18,7 +19,7 @@ pub(super) fn simplify_ec_add( solver: impl BlackBoxFunctionSolver, arguments: &[ValueId], block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { match ( dfg.get_numeric_constant(arguments[0]), @@ -56,7 +57,7 @@ pub(super) fn simplify_ec_add( let elements = im::vector![result_x, result_y, result_is_infinity]; let instruction = Instruction::MakeArray { elements, typ }; let result_array = - dfg.insert_instruction_and_results(instruction, block, None, call_stack.clone()); + dfg.insert_instruction_and_results(instruction, block, None, call_stack); SimplifyResult::SimplifiedTo(result_array.first()) } @@ -69,7 +70,7 @@ pub(super) fn simplify_msm( solver: impl BlackBoxFunctionSolver, arguments: &[ValueId], block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { let mut is_constant; @@ -149,12 +150,8 @@ pub(super) fn simplify_msm( let elements = im::vector![result_x, result_y, result_is_infinity]; let typ = Type::Array(Arc::new(vec![Type::field()]), 3); let instruction = Instruction::MakeArray { elements, typ }; - let result_array = dfg.insert_instruction_and_results( - instruction, - block, - None, - call_stack.clone(), - ); + let result_array = + dfg.insert_instruction_and_results(instruction, block, None, call_stack); return SimplifyResult::SimplifiedTo(result_array.first()); } @@ -178,13 +175,12 @@ pub(super) fn simplify_msm( // Construct the simplified MSM expression let typ = Type::Array(Arc::new(vec![Type::field()]), var_scalars.len() as u32); let scalars = Instruction::MakeArray { elements: var_scalars.into(), typ }; - let scalars = dfg - .insert_instruction_and_results(scalars, block, None, call_stack.clone()) - .first(); + let scalars = + dfg.insert_instruction_and_results(scalars, block, None, call_stack).first(); let typ = Type::Array(Arc::new(vec![Type::field()]), var_points.len() as u32); let points = Instruction::MakeArray { elements: var_points.into(), typ }; let points = - dfg.insert_instruction_and_results(points, block, None, call_stack.clone()).first(); + dfg.insert_instruction_and_results(points, block, None, call_stack).first(); let msm = dfg.import_intrinsic(Intrinsic::BlackBox(BlackBoxFunc::MultiScalarMul)); SimplifyResult::SimplifiedToInstruction(Instruction::Call { func: msm, @@ -200,7 +196,7 @@ pub(super) fn simplify_poseidon2_permutation( solver: impl BlackBoxFunctionSolver, arguments: &[ValueId], block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { match (dfg.get_array_constant(arguments[0]), dfg.get_numeric_constant(arguments[1])) { (Some((state, _)), Some(state_length)) if array_is_constant(dfg, &state) => { @@ -235,7 +231,7 @@ pub(super) fn simplify_hash( arguments: &[ValueId], hash_function: fn(&[u8]) -> Result<[u8; 32], BlackBoxResolutionError>, block: BasicBlockId, - call_stack: &CallStack, + call_stack: CallStackId, ) -> SimplifyResult { match dfg.get_array_constant(arguments[0]) { Some((input, _)) if array_is_constant(dfg, &input) => { diff --git a/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs b/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs index 348c78683a0..6936c7ad542 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs @@ -92,7 +92,7 @@ fn evaluate_assert_constant( if arguments.iter().all(|arg| function.dfg.is_constant(*arg)) { Ok(false) } else { - let call_stack = function.dfg.get_call_stack(instruction); + let call_stack = function.dfg.get_instruction_call_stack(instruction); Err(RuntimeError::AssertConstantFailed { call_stack }) } } @@ -113,14 +113,14 @@ fn evaluate_static_assert( } if !function.dfg.is_constant(arguments[1]) { - let call_stack = function.dfg.get_call_stack(instruction); + let call_stack = function.dfg.get_instruction_call_stack(instruction); return Err(RuntimeError::StaticAssertDynamicMessage { call_stack }); } if function.dfg.is_constant_true(arguments[0]) { Ok(false) } else { - let call_stack = function.dfg.get_call_stack(instruction); + let call_stack = function.dfg.get_instruction_call_stack(instruction); if function.dfg.is_constant(arguments[0]) { Err(RuntimeError::StaticAssertFailed { call_stack }) } else { diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index e039b8f0f9e..803e76fa6cb 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -315,7 +315,7 @@ impl<'brillig> Context<'brillig> { if matches!(instruction, Instruction::MakeArray { .. }) { let value = *cached.last().unwrap(); let inc_rc = Instruction::IncrementRc { value }; - let call_stack = dfg.get_call_stack(id); + let call_stack = dfg.get_instruction_call_stack_id(id); dfg.insert_instruction_and_results(inc_rc, block, None, call_stack); } @@ -418,7 +418,7 @@ impl<'brillig> Context<'brillig> { .requires_ctrl_typevars() .then(|| vecmap(old_results, |result| dfg.type_of_value(*result))); - let call_stack = dfg.get_call_stack(id); + let call_stack = dfg.get_instruction_call_stack_id(id); let new_results = match dfg.insert_instruction_and_results(instruction, block, ctrl_typevars, call_stack) { diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index f7ac6f7b313..7d364ddc81e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -1,14 +1,12 @@ //! Dead Instruction Elimination (DIE) pass: Removes any instruction without side-effects for //! which the results are unused. use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; -use im::Vector; -use noirc_errors::Location; use rayon::iter::{IntoParallelRefMutIterator, ParallelIterator}; use crate::ssa::{ ir::{ basic_block::{BasicBlock, BasicBlockId}, - dfg::DataFlowGraph, + dfg::{CallStackId, DataFlowGraph}, function::Function, instruction::{BinaryOp, Instruction, InstructionId, Intrinsic}, post_order::PostOrder, @@ -281,7 +279,7 @@ impl Context { _ => panic!("Expected an ArrayGet or ArraySet instruction here"), }; - let call_stack = function.dfg.get_call_stack(instruction_id); + let call_stack = function.dfg.get_instruction_call_stack_id(instruction_id); let (lhs, rhs) = if function.dfg.get_numeric_constant(*index).is_some() { // If we are here it means the index is known but out of bounds. That's always an error! @@ -297,7 +295,7 @@ impl Context { Instruction::Cast(*index, Type::unsigned(SSA_WORD_SIZE)), block_id, None, - call_stack.clone(), + call_stack, ); let index = index.first(); @@ -308,7 +306,7 @@ impl Context { Instruction::binary(BinaryOp::Lt, index, array_length), block_id, None, - call_stack.clone(), + call_stack, ); let is_index_out_of_bounds = is_index_out_of_bounds.first(); let true_const = function.dfg.make_constant(true.into(), Type::bool()); @@ -321,7 +319,7 @@ impl Context { rhs, function, block_id, - call_stack.clone(), + call_stack, ); let message = Some("Index out of bounds".to_owned().into()); @@ -484,7 +482,7 @@ fn apply_side_effects( rhs: ValueId, function: &mut Function, block_id: BasicBlockId, - call_stack: Vector, + call_stack: CallStackId, ) -> (ValueId, ValueId) { // See if there's an active "enable side effects" condition let Some(condition) = side_effects_condition else { @@ -499,7 +497,7 @@ fn apply_side_effects( Instruction::Cast(condition, Type::bool()), block_id, None, - call_stack.clone(), + call_stack, ); let casted_condition = casted_condition.first(); @@ -507,7 +505,7 @@ fn apply_side_effects( Instruction::binary(BinaryOp::Mul, lhs, casted_condition), block_id, None, - call_stack.clone(), + call_stack, ); let lhs = lhs.first(); diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 3fbccf93ec9..321833fd008 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -140,7 +140,7 @@ use crate::ssa::{ ir::{ basic_block::BasicBlockId, cfg::ControlFlowGraph, - dfg::{CallStack, InsertInstructionResult}, + dfg::{CallStackId, InsertInstructionResult}, function::{Function, FunctionId, RuntimeType}, function_inserter::FunctionInserter, instruction::{BinaryOp, Instruction, InstructionId, Intrinsic, TerminatorInstruction}, @@ -234,7 +234,7 @@ struct ConditionalContext { // First block of the else branch else_branch: Option, // Call stack where the final location is that of the entire `if` expression - call_stack: CallStack, + call_stack: CallStackId, } fn flatten_function_cfg(function: &mut Function, no_predicates: &HashMap) { @@ -285,7 +285,7 @@ impl<'f> Context<'f> { if let Some(context) = self.condition_stack.last() { let previous_branch = context.else_branch.as_ref().unwrap_or(&context.then_branch); let and = Instruction::binary(BinaryOp::And, previous_branch.condition, condition); - let call_stack = self.inserter.function.dfg.get_value_call_stack(condition); + let call_stack = self.inserter.function.dfg.get_value_call_stack_id(condition); self.insert_instruction(and, call_stack) } else { condition @@ -340,7 +340,7 @@ impl<'f> Context<'f> { self.insert_instruction_with_typevars( Instruction::EnableSideEffectsIf { condition: one }, None, - im::Vector::new(), + CallStackId::root(), ); self.push_instruction(*instruction); self.insert_current_side_effects_enabled(); @@ -368,13 +368,7 @@ impl<'f> Context<'f> { call_stack, } => { self.arguments_stack.push(vec![]); - self.if_start( - condition, - then_destination, - else_destination, - &block, - call_stack.clone(), - ) + self.if_start(condition, then_destination, else_destination, &block, *call_stack) } TerminatorInstruction::Jmp { destination, arguments, call_stack: _ } => { let arguments = vecmap(arguments.clone(), |value| self.inserter.resolve(value)); @@ -390,7 +384,7 @@ impl<'f> Context<'f> { } } TerminatorInstruction::Return { return_values, call_stack } => { - let call_stack = call_stack.clone(); + let call_stack = *call_stack; let return_values = vecmap(return_values.clone(), |value| self.inserter.resolve(value)); let new_return = TerminatorInstruction::Return { return_values, call_stack }; @@ -409,7 +403,7 @@ impl<'f> Context<'f> { then_destination: &BasicBlockId, else_destination: &BasicBlockId, if_entry: &BasicBlockId, - call_stack: CallStack, + call_stack: CallStackId, ) -> Vec { // manage conditions let old_condition = *condition; @@ -450,11 +444,9 @@ impl<'f> Context<'f> { cond_context.then_branch.last_block = *block; let condition_call_stack = - self.inserter.function.dfg.get_value_call_stack(cond_context.condition); - let else_condition = self.insert_instruction( - Instruction::Not(cond_context.condition), - condition_call_stack.clone(), - ); + self.inserter.function.dfg.get_value_call_stack_id(cond_context.condition); + let else_condition = + self.insert_instruction(Instruction::Not(cond_context.condition), condition_call_stack); let else_condition = self.link_condition(else_condition); let old_allocations = std::mem::take(&mut self.local_allocations); @@ -552,7 +544,7 @@ impl<'f> Context<'f> { else_condition, else_value: else_arg, }; - let call_stack = cond_context.call_stack.clone(); + let call_stack = cond_context.call_stack; self.inserter .function .dfg @@ -569,7 +561,7 @@ impl<'f> Context<'f> { /// Insert a new instruction into the function's entry block. /// Unlike push_instruction, this function will not map any ValueIds. /// within the given instruction, nor will it modify self.values in any way. - fn insert_instruction(&mut self, instruction: Instruction, call_stack: CallStack) -> ValueId { + fn insert_instruction(&mut self, instruction: Instruction, call_stack: CallStackId) -> ValueId { let block = self.inserter.function.entry_block(); self.inserter .function @@ -586,7 +578,7 @@ impl<'f> Context<'f> { &mut self, instruction: Instruction, ctrl_typevars: Option>, - call_stack: CallStack, + call_stack: CallStackId, ) -> InsertInstructionResult { let block = self.inserter.function.entry_block(); self.inserter.function.dfg.insert_instruction_and_results( @@ -610,7 +602,7 @@ impl<'f> Context<'f> { } }; let enable_side_effects = Instruction::EnableSideEffectsIf { condition }; - let call_stack = self.inserter.function.dfg.get_value_call_stack(condition); + let call_stack = self.inserter.function.dfg.get_value_call_stack_id(condition); self.insert_instruction_with_typevars(enable_side_effects, None, call_stack); } @@ -626,7 +618,7 @@ impl<'f> Context<'f> { /// any instructions in between it should be None. fn push_instruction(&mut self, id: InstructionId) { let (instruction, call_stack) = self.inserter.map_instruction(id); - let instruction = self.handle_instruction_side_effects(instruction, call_stack.clone()); + let instruction = self.handle_instruction_side_effects(instruction, call_stack); let instruction_is_allocate = matches!(&instruction, Instruction::Allocate); let entry = self.inserter.function.entry_block(); @@ -644,7 +636,7 @@ impl<'f> Context<'f> { fn handle_instruction_side_effects( &mut self, instruction: Instruction, - call_stack: CallStack, + call_stack: CallStackId, ) -> Instruction { if let Some(condition) = self.get_last_condition() { match instruction { @@ -658,12 +650,12 @@ impl<'f> Context<'f> { let casted_condition = self.insert_instruction( Instruction::Cast(condition, argument_type), - call_stack.clone(), + call_stack, ); let lhs = self.insert_instruction( Instruction::binary(BinaryOp::Mul, lhs, casted_condition), - call_stack.clone(), + call_stack, ); let rhs = self.insert_instruction( Instruction::binary(BinaryOp::Mul, rhs, casted_condition), @@ -682,15 +674,11 @@ impl<'f> Context<'f> { let typ = self.inserter.function.dfg.type_of_value(value); let load = Instruction::Load { address }; let previous_value = self - .insert_instruction_with_typevars( - load, - Some(vec![typ]), - call_stack.clone(), - ) + .insert_instruction_with_typevars(load, Some(vec![typ]), call_stack) .first(); - let else_condition = self - .insert_instruction(Instruction::Not(condition), call_stack.clone()); + let else_condition = + self.insert_instruction(Instruction::Not(condition), call_stack); let instruction = Instruction::IfElse { then_condition: condition, @@ -710,12 +698,12 @@ impl<'f> Context<'f> { let argument_type = self.inserter.function.dfg.type_of_value(value); let casted_condition = self.insert_instruction( Instruction::Cast(condition, argument_type), - call_stack.clone(), + call_stack, ); let value = self.insert_instruction( Instruction::binary(BinaryOp::Mul, value, casted_condition), - call_stack.clone(), + call_stack, ); Instruction::RangeCheck { value, max_bit_size, assert_message } } @@ -727,11 +715,11 @@ impl<'f> Context<'f> { let casted_condition = self.insert_instruction( Instruction::Cast(condition, argument_type), - call_stack.clone(), + call_stack, ); let field = self.insert_instruction( Instruction::binary(BinaryOp::Mul, field, casted_condition), - call_stack.clone(), + call_stack, ); arguments[0] = field; @@ -740,8 +728,8 @@ impl<'f> Context<'f> { } //Issue #5045: We set curve points to infinity if condition is false Value::Intrinsic(Intrinsic::BlackBox(BlackBoxFunc::EmbeddedCurveAdd)) => { - arguments[2] = self.var_or_one(arguments[2], condition, call_stack.clone()); - arguments[5] = self.var_or_one(arguments[5], condition, call_stack.clone()); + arguments[2] = self.var_or_one(arguments[2], condition, call_stack); + arguments[5] = self.var_or_one(arguments[5], condition, call_stack); Instruction::Call { func, arguments } } @@ -759,7 +747,7 @@ impl<'f> Context<'f> { let (elements, typ) = self.apply_predicate_to_msm_argument( arguments[points_array_idx], condition, - call_stack.clone(), + call_stack, ); let instruction = Instruction::MakeArray { elements, typ }; @@ -783,7 +771,7 @@ impl<'f> Context<'f> { &mut self, argument: ValueId, predicate: ValueId, - call_stack: CallStack, + call_stack: CallStackId, ) -> (im::Vector, Type) { let array_typ; let mut array_with_predicate = im::Vector::new(); @@ -791,11 +779,7 @@ impl<'f> Context<'f> { array_typ = typ.clone(); for (i, value) in array.clone().iter().enumerate() { if i % 3 == 2 { - array_with_predicate.push_back(self.var_or_one( - *value, - predicate, - call_stack.clone(), - )); + array_with_predicate.push_back(self.var_or_one(*value, predicate, call_stack)); } else { array_with_predicate.push_back(*value); } @@ -811,13 +795,10 @@ impl<'f> Context<'f> { } // Computes: if condition { var } else { 1 } - fn var_or_one(&mut self, var: ValueId, condition: ValueId, call_stack: CallStack) -> ValueId { - let field = self.insert_instruction( - Instruction::binary(BinaryOp::Mul, var, condition), - call_stack.clone(), - ); - let not_condition = - self.insert_instruction(Instruction::Not(condition), call_stack.clone()); + fn var_or_one(&mut self, var: ValueId, condition: ValueId, call_stack: CallStackId) -> ValueId { + let field = + self.insert_instruction(Instruction::binary(BinaryOp::Mul, var, condition), call_stack); + let not_condition = self.insert_instruction(Instruction::Not(condition), call_stack); self.insert_instruction( Instruction::binary(BinaryOp::Add, field, not_condition), call_stack, diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs index 6ea235b9414..add76a659b8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs @@ -3,7 +3,7 @@ use fxhash::{FxHashMap as HashMap, FxHashSet}; use crate::ssa::ir::{ basic_block::BasicBlockId, - dfg::{CallStack, DataFlowGraph, InsertInstructionResult}, + dfg::{CallStackId, DataFlowGraph, InsertInstructionResult}, instruction::{BinaryOp, Instruction}, types::Type, value::{Value, ValueId}, @@ -21,7 +21,7 @@ pub(crate) struct ValueMerger<'a> { array_set_conditionals: &'a mut HashMap, - call_stack: CallStack, + call_stack: CallStackId, } impl<'a> ValueMerger<'a> { @@ -31,7 +31,7 @@ impl<'a> ValueMerger<'a> { slice_sizes: &'a mut HashMap, array_set_conditionals: &'a mut HashMap, current_condition: Option, - call_stack: CallStack, + call_stack: CallStackId, ) -> Self { ValueMerger { dfg, @@ -106,10 +106,10 @@ impl<'a> ValueMerger<'a> { return then_value; } - let then_call_stack = dfg.get_value_call_stack(then_value); - let else_call_stack = dfg.get_value_call_stack(else_value); + let then_call_stack = dfg.get_value_call_stack_id(then_value); + let else_call_stack = dfg.get_value_call_stack_id(else_value); - let call_stack = if then_call_stack.is_empty() { else_call_stack } else { then_call_stack }; + let call_stack = if then_call_stack.is_root() { else_call_stack } else { then_call_stack }; // We must cast the bool conditions to the actual numeric type used by each value. let then_condition = dfg @@ -117,7 +117,7 @@ impl<'a> ValueMerger<'a> { Instruction::Cast(then_condition, then_type), block, None, - call_stack.clone(), + call_stack, ) .first(); let else_condition = dfg @@ -125,17 +125,15 @@ impl<'a> ValueMerger<'a> { Instruction::Cast(else_condition, else_type), block, None, - call_stack.clone(), + call_stack, ) .first(); let mul = Instruction::binary(BinaryOp::Mul, then_condition, then_value); - let then_value = - dfg.insert_instruction_and_results(mul, block, None, call_stack.clone()).first(); + let then_value = dfg.insert_instruction_and_results(mul, block, None, call_stack).first(); let mul = Instruction::binary(BinaryOp::Mul, else_condition, else_value); - let else_value = - dfg.insert_instruction_and_results(mul, block, None, call_stack.clone()).first(); + let else_value = dfg.insert_instruction_and_results(mul, block, None, call_stack).first(); let add = Instruction::binary(BinaryOp::Add, then_value, else_value); dfg.insert_instruction_and_results(add, block, None, call_stack).first() @@ -182,12 +180,7 @@ impl<'a> ValueMerger<'a> { let mut get_element = |array, typevars| { let get = Instruction::ArrayGet { array, index }; self.dfg - .insert_instruction_and_results( - get, - self.block, - typevars, - self.call_stack.clone(), - ) + .insert_instruction_and_results(get, self.block, typevars, self.call_stack) .first() }; @@ -204,8 +197,9 @@ impl<'a> ValueMerger<'a> { } let instruction = Instruction::MakeArray { elements: merged, typ }; - let call_stack = self.call_stack.clone(); - self.dfg.insert_instruction_and_results(instruction, self.block, None, call_stack).first() + self.dfg + .insert_instruction_and_results(instruction, self.block, None, self.call_stack) + .first() } fn merge_slice_values( @@ -259,7 +253,7 @@ impl<'a> ValueMerger<'a> { get, self.block, typevars, - self.call_stack.clone(), + self.call_stack, ) .first() } @@ -283,7 +277,7 @@ impl<'a> ValueMerger<'a> { } let instruction = Instruction::MakeArray { elements: merged, typ }; - let call_stack = self.call_stack.clone(); + let call_stack = self.call_stack; self.dfg.insert_instruction_and_results(instruction, self.block, None, call_stack).first() } @@ -305,7 +299,7 @@ impl<'a> ValueMerger<'a> { } } let instruction = Instruction::MakeArray { elements: array, typ: typ.clone() }; - let call_stack = self.call_stack.clone(); + let call_stack = self.call_stack; self.dfg .insert_instruction_and_results(instruction, self.block, None, call_stack) .first() @@ -399,12 +393,7 @@ impl<'a> ValueMerger<'a> { let mut get_element = |array, typevars| { let get = Instruction::ArrayGet { array, index }; self.dfg - .insert_instruction_and_results( - get, - self.block, - typevars, - self.call_stack.clone(), - ) + .insert_instruction_and_results(get, self.block, typevars, self.call_stack) .first() }; @@ -423,12 +412,7 @@ impl<'a> ValueMerger<'a> { } fn insert_instruction(&mut self, instruction: Instruction) -> InsertInstructionResult { - self.dfg.insert_instruction_and_results( - instruction, - self.block, - None, - self.call_stack.clone(), - ) + self.dfg.insert_instruction_and_results(instruction, self.block, None, self.call_stack) } fn insert_array_set( @@ -439,12 +423,8 @@ impl<'a> ValueMerger<'a> { condition: Option, ) -> InsertInstructionResult { let instruction = Instruction::ArraySet { array, index, value, mutable: false }; - let result = self.dfg.insert_instruction_and_results( - instruction, - self.block, - None, - self.call_stack.clone(), - ); + let result = + self.dfg.insert_instruction_and_results(instruction, self.block, None, self.call_stack); if let Some(condition) = condition { let result_index = if result.len() == 1 { diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index f91487fd73e..bdaf49c7c22 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -11,7 +11,7 @@ use crate::ssa::{ function_builder::FunctionBuilder, ir::{ basic_block::BasicBlockId, - dfg::{CallStack, InsertInstructionResult}, + dfg::{CallStackId, InsertInstructionResult}, function::{Function, FunctionId, RuntimeType}, instruction::{Instruction, InstructionId, TerminatorInstruction}, value::{Value, ValueId}, @@ -83,7 +83,7 @@ struct InlineContext { recursion_level: u32, builder: FunctionBuilder, - call_stack: CallStack, + call_stack: CallStackId, // The FunctionId of the entry point function we're inlining into in the old, unmodified Ssa. entry_point: FunctionId, @@ -365,7 +365,7 @@ impl InlineContext { builder, recursion_level: 0, entry_point, - call_stack: CallStack::new(), + call_stack: CallStackId::root(), inline_no_predicates_functions, functions_not_to_inline, } @@ -660,13 +660,23 @@ impl<'function> PerFunctionContext<'function> { let old_results = self.source_function.dfg.instruction_results(call_id); let arguments = vecmap(arguments, |arg| self.translate_value(*arg)); - let call_stack = self.source_function.dfg.get_call_stack(call_id); + let call_stack = self.source_function.dfg.get_instruction_call_stack(call_id); let call_stack_len = call_stack.len(); - self.context.call_stack.append(call_stack); - + let new_call_stack = self + .context + .builder + .current_function + .dfg + .extend_call_stack(self.context.call_stack, &call_stack); + + self.context.call_stack = new_call_stack; let new_results = self.context.inline_function(ssa, function, &arguments); - - self.context.call_stack.truncate(self.context.call_stack.len() - call_stack_len); + self.context.call_stack = self + .context + .builder + .current_function + .dfg + .unwind_call_stack(self.context.call_stack, call_stack_len); let new_results = InsertInstructionResult::Results(call_id, &new_results); Self::insert_new_instruction_results(&mut self.values, old_results, new_results); @@ -677,9 +687,14 @@ impl<'function> PerFunctionContext<'function> { fn push_instruction(&mut self, id: InstructionId) { let instruction = self.source_function.dfg[id].map_values(|id| self.translate_value(id)); - let mut call_stack = self.context.call_stack.clone(); - call_stack.append(self.source_function.dfg.get_call_stack(id)); - + let mut call_stack = self.context.call_stack; + let source_call_stack = self.source_function.dfg.get_instruction_call_stack(id); + call_stack = self + .context + .builder + .current_function + .dfg + .extend_call_stack(call_stack, &source_call_stack); let results = self.source_function.dfg.instruction_results(id); let results = vecmap(results, |id| self.source_function.dfg.resolve(*id)); @@ -736,8 +751,13 @@ impl<'function> PerFunctionContext<'function> { let destination = self.translate_block(*destination, block_queue); let arguments = vecmap(arguments, |arg| self.translate_value(*arg)); - let mut new_call_stack = self.context.call_stack.clone(); - new_call_stack.append(call_stack.clone()); + let call_stack = self.source_function.dfg.get_call_stack(*call_stack); + let new_call_stack = self + .context + .builder + .current_function + .dfg + .extend_call_stack(self.context.call_stack, &call_stack); self.context .builder @@ -752,9 +772,13 @@ impl<'function> PerFunctionContext<'function> { call_stack, } => { let condition = self.translate_value(*condition); - - let mut new_call_stack = self.context.call_stack.clone(); - new_call_stack.append(call_stack.clone()); + let call_stack = self.source_function.dfg.get_call_stack(*call_stack); + let new_call_stack = self + .context + .builder + .current_function + .dfg + .extend_call_stack(self.context.call_stack, &call_stack); // See if the value of the condition is known, and if so only inline the reachable // branch. This lets us inline some recursive functions without recurring forever. @@ -791,8 +815,14 @@ impl<'function> PerFunctionContext<'function> { let block_id = self.context.builder.current_block(); if self.inlining_entry { - let mut new_call_stack = self.context.call_stack.clone(); - new_call_stack.append(call_stack.clone()); + let call_stack = self.source_function.dfg.get_call_stack(*call_stack); + let new_call_stack = self + .context + .builder + .current_function + .dfg + .extend_call_stack(self.context.call_stack, &call_stack); + self.context .builder .set_call_stack(new_call_stack) diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index a5b60fb5fcd..44d016837df 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -86,7 +86,9 @@ impl Context { let instruction = old_function.dfg[old_instruction_id] .map_values(|value| self.new_ids.map_value(new_function, old_function, value)); - let call_stack = old_function.dfg.get_call_stack(old_instruction_id); + let call_stack = old_function.dfg.get_instruction_call_stack_id(old_instruction_id); + let locations = old_function.dfg.get_call_stack(call_stack); + let new_call_stack = new_function.dfg.get_or_insert_locations(locations); let old_results = old_function.dfg.instruction_results(old_instruction_id); let ctrl_typevars = instruction @@ -97,7 +99,7 @@ impl Context { instruction, new_block_id, ctrl_typevars, - call_stack, + new_call_stack, ); assert_eq!(old_results.len(), new_results.len()); @@ -113,6 +115,9 @@ impl Context { .take_terminator() .map_values(|value| self.new_ids.map_value(new_function, old_function, value)); terminator.mutate_blocks(|old_block| self.new_ids.blocks[&old_block]); + let locations = old_function.dfg.get_call_stack(terminator.call_stack()); + let new_call_stack = new_function.dfg.get_or_insert_locations(locations); + terminator.set_call_stack(new_call_stack); new_function.dfg.set_block_terminator(new_block_id, terminator); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index ccf5bd9d9f8..37f963a045e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -5,7 +5,7 @@ use acvm::{acir::AcirField, FieldElement}; use crate::ssa::{ ir::{ basic_block::BasicBlockId, - dfg::{CallStack, InsertInstructionResult}, + dfg::{CallStackId, InsertInstructionResult}, function::{Function, RuntimeType}, instruction::{Binary, BinaryOp, Endian, Instruction, InstructionId, Intrinsic}, types::{NumericType, Type}, @@ -40,7 +40,7 @@ impl Function { function: self, new_instructions: Vec::new(), block, - call_stack: CallStack::default(), + call_stack: CallStackId::root(), }; context.remove_bit_shifts(); @@ -52,7 +52,7 @@ struct Context<'f> { new_instructions: Vec, block: BasicBlockId, - call_stack: CallStack, + call_stack: CallStackId, } impl Context<'_> { @@ -64,7 +64,8 @@ impl Context<'_> { Instruction::Binary(Binary { lhs, rhs, operator }) if matches!(operator, BinaryOp::Shl | BinaryOp::Shr) => { - self.call_stack = self.function.dfg.get_call_stack(instruction_id).clone(); + self.call_stack = + self.function.dfg.get_instruction_call_stack_id(instruction_id); let old_result = *self.function.dfg.instruction_results(instruction_id).first().unwrap(); @@ -295,7 +296,7 @@ impl Context<'_> { instruction, self.block, ctrl_typevars, - self.call_stack.clone(), + self.call_stack, ); if let InsertInstructionResult::Results(instruction_id, _) = result { diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs index 02191801fcd..f509eec04e8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs @@ -75,7 +75,7 @@ impl Context { let typ = function.dfg.type_of_value(then_value); assert!(!matches!(typ, Type::Numeric(_))); - let call_stack = function.dfg.get_call_stack(instruction); + let call_stack = function.dfg.get_instruction_call_stack_id(instruction); let mut value_merger = ValueMerger::new( &mut function.dfg, block, diff --git a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs index e7f8d227d28..22fdf0a7987 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs @@ -109,7 +109,7 @@ fn check_for_constant_jmpif( }; let arguments = Vec::new(); - let call_stack = call_stack.clone(); + let call_stack = *call_stack; let jmp = TerminatorInstruction::Jmp { destination, arguments, call_stack }; function.dfg[block].set_terminator(jmp); cfg.recompute_block(function, block); @@ -223,7 +223,7 @@ fn check_for_negated_jmpif_condition( { if let Value::Instruction { instruction, .. } = function.dfg[*condition] { if let Instruction::Not(negated_condition) = function.dfg[instruction] { - let call_stack = call_stack.clone(); + let call_stack = *call_stack; let jmpif = TerminatorInstruction::JmpIf { condition: negated_condition, then_destination: *else_destination, diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index 22daba1de45..d48442ed0f3 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -28,7 +28,7 @@ use crate::{ ir::{ basic_block::BasicBlockId, cfg::ControlFlowGraph, - dfg::{CallStack, DataFlowGraph}, + dfg::{CallStack, CallStackId, DataFlowGraph}, dom::DominatorTree, function::Function, function_inserter::{ArrayCache, FunctionInserter}, @@ -469,7 +469,7 @@ impl Loop { match context.dfg()[fresh_block].unwrap_terminator() { TerminatorInstruction::JmpIf { condition, then_destination, else_destination, call_stack } => { let condition = *condition; - let next_blocks = context.handle_jmpif(condition, *then_destination, *else_destination, call_stack.clone()); + let next_blocks = context.handle_jmpif(condition, *then_destination, *else_destination, *call_stack); // If there is only 1 next block the jmpif evaluated to a single known block. // This is the expected case and lets us know if we should loop again or not. @@ -757,10 +757,11 @@ fn get_induction_variable(function: &Function, block: BasicBlockId) -> Result Err(terminator.call_stack()), + Some(terminator) => Err(function.dfg.get_call_stack(terminator.call_stack())), None => Err(CallStack::new()), } } @@ -859,12 +860,7 @@ impl<'f> LoopIteration<'f> { then_destination, else_destination, call_stack, - } => self.handle_jmpif( - *condition, - *then_destination, - *else_destination, - call_stack.clone(), - ), + } => self.handle_jmpif(*condition, *then_destination, *else_destination, *call_stack), TerminatorInstruction::Jmp { destination, arguments, call_stack: _ } => { if self.get_original_block(*destination) == self.loop_.header { // We found the back-edge of the loop. @@ -888,7 +884,7 @@ impl<'f> LoopIteration<'f> { condition: ValueId, then_destination: BasicBlockId, else_destination: BasicBlockId, - call_stack: CallStack, + call_stack: CallStackId, ) -> Vec { let condition = self.inserter.resolve(condition); diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 91a49018f76..3a2b9e489d7 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -90,12 +90,14 @@ pub(crate) fn generate_ssa( None, ); } + let return_call_stack = + function_context.builder.current_function.dfg.add_location_to_root(return_location); let return_instruction = function_context.builder.current_function.dfg[block].unwrap_terminator_mut(); + match return_instruction { TerminatorInstruction::Return { return_values, call_stack } => { - call_stack.clear(); - call_stack.push_back(return_location); + *call_stack = return_call_stack; // replace the returned values with the return data array if let Some(return_data_bus) = return_data.databus { return_values.clear(); From 32554003945839ec5b5412419370eba5c055fe71 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 12 Dec 2024 13:53:41 +0000 Subject: [PATCH 02/13] fix merge issues --- .../src/ssa/checks/check_for_underconstrained_values.rs | 2 +- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 7 ++++--- compiler/noirc_evaluator/src/ssa/opt/die.rs | 3 +-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs index 49097c8c9cc..b0a62d9cb65 100644 --- a/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs +++ b/compiler/noirc_evaluator/src/ssa/checks/check_for_underconstrained_values.rs @@ -387,7 +387,7 @@ impl DependencyContext { .keys() .map(|brillig_call| { SsaReport::Bug(InternalBug::UncheckedBrilligCall { - call_stack: function.dfg.get_call_stack(*brillig_call), + call_stack: function.dfg.get_instruction_call_stack(*brillig_call), }) }) .collect(); diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 408cf785528..8484da767d5 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -8,6 +8,7 @@ use super::{ instruction::{ Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction, }, + list::List, map::DenseMap, types::{NumericType, Type}, value::{Value, ValueId}, @@ -43,10 +44,10 @@ impl CallStackId { // Retrieves a CallStack from a CallStackId fn get_call_stack(&self, locations: &[LocationNode]) -> CallStack { - let mut call_stack = im::Vector::new(); + let mut call_stack = List::new(); let mut current_location = *self; while let Some(parent) = locations[current_location.index()].parent { - call_stack.push_front(locations[current_location.index()].value); + call_stack.push_back(locations[current_location.index()].value); current_location = parent; } call_stack @@ -168,7 +169,7 @@ pub(crate) struct DataFlowGraph { pub(crate) data_bus: DataBus, } -pub(crate) type CallStack = super::list::List; +pub(crate) type CallStack = List; impl DataFlowGraph { /// Creates a new basic block with no parameters. diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index c2d6e98241d..66a0d8a349e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -494,8 +494,7 @@ fn apply_side_effects( // Condition needs to be cast to argument type in order to multiply them together. // In our case, lhs is always a boolean. let cast = Instruction::Cast(condition, NumericType::bool()); - let casted_condition = - dfg.insert_instruction_and_results(cast, block_id, None, call_stack); + let casted_condition = dfg.insert_instruction_and_results(cast, block_id, None, call_stack); let casted_condition = casted_condition.first(); let lhs = dfg.insert_instruction_and_results( From 23f6af1329a609bba3494f3d3876f49dc6ab17d8 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 12 Dec 2024 14:10:47 +0000 Subject: [PATCH 03/13] fix unit tests --- compiler/noirc_evaluator/src/acir/mod.rs | 3 ++- .../src/ssa/function_builder/mod.rs | 2 +- compiler/noirc_evaluator/src/ssa/ir/cfg.rs | 16 ++++++++-------- compiler/noirc_evaluator/src/ssa/ir/dom.rs | 7 +++++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index ef04571abdb..7a80278b114 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -2933,7 +2933,8 @@ mod test { // Set a call stack for testing whether `brillig_locations` in the `GeneratedAcir` was accurately set. let mut stack = CallStack::unit(Location::dummy()); stack.push_back(Location::dummy()); - builder.set_call_stack(stack); + let call_stack = builder.current_function.dfg.get_or_insert_locations(stack); + builder.set_call_stack(call_stack); let foo_v0 = builder.add_parameter(Type::field()); let foo_v1 = builder.add_parameter(Type::field()); diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 292da99be33..a36366d01d9 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -34,7 +34,7 @@ use super::{ /// Contrary to the name, this struct has the capacity to build as many /// functions as needed, although it is limited to one function at a time. pub(crate) struct FunctionBuilder { - pub(super) current_function: Function, + pub(crate) current_function: Function, current_block: BasicBlockId, finished_functions: Vec, call_stack: CallStackId, diff --git a/compiler/noirc_evaluator/src/ssa/ir/cfg.rs b/compiler/noirc_evaluator/src/ssa/ir/cfg.rs index 2268e6b2191..fc4d82c092d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/cfg.rs @@ -128,7 +128,7 @@ impl ControlFlowGraph { #[cfg(test)] mod tests { use crate::ssa::ir::{ - dfg::CallStack, instruction::TerminatorInstruction, map::Id, types::Type, + dfg::CallStackId, instruction::TerminatorInstruction, map::Id, types::Type, }; use super::{super::function::Function, ControlFlowGraph}; @@ -140,7 +140,7 @@ mod tests { let block_id = func.entry_block(); func.dfg[block_id].set_terminator(TerminatorInstruction::Return { return_values: vec![], - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }); ControlFlowGraph::with_function(&func); @@ -168,17 +168,17 @@ mod tests { condition: cond, then_destination: block2_id, else_destination: block1_id, - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }); func.dfg[block1_id].set_terminator(TerminatorInstruction::JmpIf { condition: cond, then_destination: block1_id, else_destination: block2_id, - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }); func.dfg[block2_id].set_terminator(TerminatorInstruction::Return { return_values: vec![], - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }); let mut cfg = ControlFlowGraph::with_function(&func); @@ -226,18 +226,18 @@ mod tests { let ret_block_id = func.dfg.make_block(); func.dfg[ret_block_id].set_terminator(TerminatorInstruction::Return { return_values: vec![], - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }); func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp { destination: ret_block_id, arguments: vec![], - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }); func.dfg[block0_id].set_terminator(TerminatorInstruction::JmpIf { condition: cond, then_destination: block1_id, else_destination: ret_block_id, - call_stack: CallStack::new(), + call_stack: CallStackId::root(), }); // Recompute new and changed blocks diff --git a/compiler/noirc_evaluator/src/ssa/ir/dom.rs b/compiler/noirc_evaluator/src/ssa/ir/dom.rs index 504eecf4801..2b0bb200843 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dom.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dom.rs @@ -272,7 +272,7 @@ mod tests { use crate::ssa::{ function_builder::FunctionBuilder, ir::{ - basic_block::BasicBlockId, dfg::CallStack, dom::DominatorTree, function::Function, + basic_block::BasicBlockId, dfg::CallStackId, dom::DominatorTree, function::Function, instruction::TerminatorInstruction, map::Id, types::Type, }, }; @@ -284,7 +284,10 @@ mod tests { let block0_id = func.entry_block(); func.dfg.set_block_terminator( block0_id, - TerminatorInstruction::Return { return_values: vec![], call_stack: CallStack::new() }, + TerminatorInstruction::Return { + return_values: vec![], + call_stack: CallStackId::root(), + }, ); let mut dom_tree = DominatorTree::with_function(&func); assert!(dom_tree.dominates(block0_id, block0_id)); From 647cecdcb79dcdced139d95062c000c53988f6ef Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 12 Dec 2024 16:43:38 +0000 Subject: [PATCH 04/13] Code review --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 26 +++++++++------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 8484da767d5..51a9222d184 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -42,7 +42,7 @@ impl CallStackId { self.0 == 0 } - // Retrieves a CallStack from a CallStackId + /// Construct a CallStack from a CallStackId fn get_call_stack(&self, locations: &[LocationNode]) -> CallStack { let mut call_stack = List::new(); let mut current_location = *self; @@ -53,8 +53,8 @@ impl CallStackId { call_stack } - // Adds a location to the call stack - fn add_location(&self, location: Location, locations: &mut Vec) -> CallStackId { + /// Adds a location to the call stack + fn add_child(&self, location: Location, locations: &mut Vec) -> CallStackId { if let Some(result) = locations[self.index()] .children .iter() @@ -68,11 +68,11 @@ impl CallStackId { new_location } - // Returns a new CallStackId which extends the current one with the provided call_stack. + /// Returns a new CallStackId which extends the current one with the provided call_stack. fn extend(&self, call_stack: &CallStack, locations: &mut Vec) -> CallStackId { let mut result = *self; for location in call_stack { - result = result.add_location(*location, locations); + result = result.add_child(*location, locations); } result } @@ -85,12 +85,6 @@ struct LocationNode { value: Location, } -impl LocationNode { - fn new() -> Self { - Self { parent: None, children: vec![], value: Location::dummy() } - } -} - /// The DataFlowGraph contains most of the actual data in a function including /// its blocks, instructions, and values. This struct is largely responsible for /// owning most data in a function and handing out Ids to this data that can be @@ -559,7 +553,7 @@ impl DataFlowGraph { } pub(crate) fn get_instruction_call_stack(&self, instruction: InstructionId) -> CallStack { - let call_stack = self.locations.get(&instruction).cloned().unwrap_or_default(); + let call_stack = self.get_instruction_call_stack_id(instruction); self.get_call_stack(call_stack) } @@ -573,7 +567,7 @@ impl DataFlowGraph { location: Location, ) { let call_stack = self.locations.entry(instruction).or_default(); - call_stack.add_location(location, &mut self.location_array); + call_stack.add_child(location, &mut self.location_array); } pub(crate) fn add_location_to_root(&mut self, location: Location) -> CallStackId { @@ -585,15 +579,15 @@ impl DataFlowGraph { }); CallStackId::root() } else { - CallStackId::root().add_location(location, &mut self.location_array) + CallStackId::root().add_child(location, &mut self.location_array) } } - // Get (or create) a CallStackId corresponding to the given locations + /// Get (or create) a CallStackId corresponding to the given locations pub(crate) fn get_or_insert_locations(&mut self, locations: CallStack) -> CallStackId { let mut result = CallStackId::root(); for location in locations { - result = result.add_location(location, &mut self.location_array); + result = result.add_child(location, &mut self.location_array); } result } From 456eae5fe72e2d503b94ae342afcead3b69c0cba Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 13 Dec 2024 15:11:43 +0000 Subject: [PATCH 05/13] simple fix for the performance issue --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 51a9222d184..e65c9e871f3 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -57,7 +57,7 @@ impl CallStackId { fn add_child(&self, location: Location, locations: &mut Vec) -> CallStackId { if let Some(result) = locations[self.index()] .children - .iter() + .iter().rev().take(1000) .find(|child| locations[child.index()].value == location) { return *result; From 58a0fa8d58b3c7257c35311a5bd1af477ee9fb7e Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 13 Dec 2024 15:15:31 +0000 Subject: [PATCH 06/13] format --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index e65c9e871f3..c5866a6366d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -57,7 +57,9 @@ impl CallStackId { fn add_child(&self, location: Location, locations: &mut Vec) -> CallStackId { if let Some(result) = locations[self.index()] .children - .iter().rev().take(1000) + .iter() + .rev() + .take(1000) .find(|child| locations[child.index()].value == location) { return *result; From 79793d0c876ece87387532207e6b5010e846afad Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 13 Dec 2024 15:17:10 +0000 Subject: [PATCH 07/13] fix merge issue --- compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index 7e4546083b8..44c0eb380c2 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -119,7 +119,11 @@ impl<'f> LoopInvariantContext<'f> { let result = self.inserter.function.dfg.instruction_results(instruction_id)[0]; let inc_rc = Instruction::IncrementRc { value: result }; - let call_stack = self.inserter.function.dfg.get_call_stack(instruction_id); + let call_stack = self + .inserter + .function + .dfg + .get_instruction_call_stack_id(instruction_id); self.inserter .function .dfg From 79a229d443ea49f519cb77368e089bbb73faec9a Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 13 Dec 2024 15:20:36 +0000 Subject: [PATCH 08/13] format --- compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 0676ed7b7b2..bdb7d316a1a 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -495,12 +495,7 @@ fn simplify_slice_pop_back( let element_type = Some(vec![element_type.clone()]); let get_last_elem = dfg - .insert_instruction_and_results( - get_last_elem_instr, - block, - element_type, - call_stack, - ) + .insert_instruction_and_results(get_last_elem_instr, block, element_type, call_stack) .first(); results.push_front(get_last_elem); From 523aa14dac02c750e61e9a6b4e83661223da3760 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 13 Dec 2024 17:11:09 +0000 Subject: [PATCH 09/13] switch callstack to Vector --- .../noirc_evaluator/src/acir/acir_variable.rs | 2 +- .../src/acir/generated_acir.rs | 2 +- compiler/noirc_evaluator/src/acir/mod.rs | 3 +- .../src/brillig/brillig_gen.rs | 2 +- .../src/brillig/brillig_gen/brillig_block.rs | 2 +- .../noirc_evaluator/src/brillig/brillig_ir.rs | 2 +- .../src/brillig/brillig_ir/artifact.rs | 2 +- compiler/noirc_evaluator/src/errors.rs | 2 +- .../src/ssa/function_builder/mod.rs | 3 +- .../noirc_evaluator/src/ssa/ir/basic_block.rs | 2 +- .../noirc_evaluator/src/ssa/ir/call_stack.rs | 78 ++++++++ compiler/noirc_evaluator/src/ssa/ir/cfg.rs | 2 +- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 69 +------ compiler/noirc_evaluator/src/ssa/ir/dom.rs | 4 +- .../src/ssa/ir/function_inserter.rs | 2 +- .../noirc_evaluator/src/ssa/ir/instruction.rs | 3 +- .../src/ssa/ir/instruction/call.rs | 3 +- .../src/ssa/ir/instruction/call/blackbox.rs | 2 +- compiler/noirc_evaluator/src/ssa/ir/list.rs | 187 ------------------ compiler/noirc_evaluator/src/ssa/ir/mod.rs | 2 +- compiler/noirc_evaluator/src/ssa/opt/die.rs | 3 +- .../src/ssa/opt/flatten_cfg.rs | 3 +- .../src/ssa/opt/flatten_cfg/value_merger.rs | 3 +- .../noirc_evaluator/src/ssa/opt/inlining.rs | 3 +- .../src/ssa/opt/remove_bit_shifts.rs | 3 +- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 3 +- 26 files changed, 113 insertions(+), 279 deletions(-) create mode 100644 compiler/noirc_evaluator/src/ssa/ir/call_stack.rs delete mode 100644 compiler/noirc_evaluator/src/ssa/ir/list.rs diff --git a/compiler/noirc_evaluator/src/acir/acir_variable.rs b/compiler/noirc_evaluator/src/acir/acir_variable.rs index 9f2c649ee3e..78188ea2b65 100644 --- a/compiler/noirc_evaluator/src/acir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/acir/acir_variable.rs @@ -23,7 +23,7 @@ use std::{borrow::Cow, hash::Hash}; use crate::brillig::brillig_ir::artifact::GeneratedBrillig; use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport}; use crate::ssa::ir::{ - dfg::CallStack, instruction::Endian, types::NumericType, types::Type as SsaType, + call_stack::CallStack, instruction::Endian, types::NumericType, types::Type as SsaType, }; use super::big_int::BigIntContext; diff --git a/compiler/noirc_evaluator/src/acir/generated_acir.rs b/compiler/noirc_evaluator/src/acir/generated_acir.rs index 3b29c0319ab..b6a5a817ea7 100644 --- a/compiler/noirc_evaluator/src/acir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/acir/generated_acir.rs @@ -16,7 +16,7 @@ use super::brillig_directive; use crate::{ brillig::brillig_ir::artifact::GeneratedBrillig, errors::{InternalError, RuntimeError, SsaReport}, - ssa::ir::dfg::CallStack, + ssa::ir::call_stack::CallStack, ErrorType, }; diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 7a80278b114..74a72bebc1f 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -35,7 +35,8 @@ use crate::ssa::ir::instruction::Hint; use crate::ssa::{ function_builder::data_bus::DataBus, ir::{ - dfg::{CallStack, DataFlowGraph}, + call_stack::CallStack, + dfg::DataFlowGraph, function::{Function, FunctionId, RuntimeType}, instruction::{ Binary, BinaryOp, ConstrainError, Instruction, InstructionId, Intrinsic, diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs index ca4e783aa93..5a81c79ae0d 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen.rs @@ -18,7 +18,7 @@ use super::{ }; use crate::{ errors::InternalError, - ssa::ir::{dfg::CallStack, function::Function}, + ssa::ir::{call_stack::CallStack, function::Function}, }; /// Converting an SSA function into Brillig bytecode. diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index bf7c6b324be..6393967b012 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -7,7 +7,7 @@ use crate::brillig::brillig_ir::registers::Stack; use crate::brillig::brillig_ir::{ BrilligBinaryOp, BrilligContext, ReservedRegisters, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, }; -use crate::ssa::ir::dfg::CallStack; +use crate::ssa::ir::call_stack::CallStack; use crate::ssa::ir::instruction::{ConstrainError, Hint}; use crate::ssa::ir::{ basic_block::BasicBlockId, diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 8d5f14cee94..3c100d229a6 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -30,7 +30,7 @@ pub(crate) use instructions::BrilligBinaryOp; use registers::{RegisterAllocator, ScratchSpace}; use self::{artifact::BrilligArtifact, debug_show::DebugToString, registers::Stack}; -use crate::ssa::ir::dfg::CallStack; +use crate::ssa::ir::call_stack::CallStack; use acvm::{ acir::brillig::{MemoryAddress, Opcode as BrilligOpcode}, AcirField, diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index 0e5b72c0b85..be4a6b84bc1 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -2,7 +2,7 @@ use acvm::acir::brillig::Opcode as BrilligOpcode; use acvm::acir::circuit::ErrorSelector; use std::collections::{BTreeMap, HashMap}; -use crate::ssa::ir::{basic_block::BasicBlockId, dfg::CallStack, function::FunctionId}; +use crate::ssa::ir::{basic_block::BasicBlockId, call_stack::CallStack, function::FunctionId}; use crate::ErrorType; use super::procedures::ProcedureId; diff --git a/compiler/noirc_evaluator/src/errors.rs b/compiler/noirc_evaluator/src/errors.rs index bb224617994..ee662c50b75 100644 --- a/compiler/noirc_evaluator/src/errors.rs +++ b/compiler/noirc_evaluator/src/errors.rs @@ -12,7 +12,7 @@ use iter_extended::vecmap; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; -use crate::ssa::ir::{dfg::CallStack, types::NumericType}; +use crate::ssa::ir::{call_stack::CallStack, types::NumericType}; use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Clone, Error)] diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index a36366d01d9..0c58f27447e 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -18,7 +18,8 @@ use crate::ssa::ir::{ use super::{ ir::{ basic_block::BasicBlock, - dfg::{CallStack, CallStackId, InsertInstructionResult}, + call_stack::{CallStack, CallStackId}, + dfg::InsertInstructionResult, function::RuntimeType, instruction::{ConstrainError, InstructionId, Intrinsic}, types::NumericType, diff --git a/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs b/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs index 069e1f49164..b2a923c6a51 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/basic_block.rs @@ -1,5 +1,5 @@ use super::{ - dfg::CallStackId, + call_stack::CallStackId, instruction::{InstructionId, TerminatorInstruction}, map::Id, value::ValueId, diff --git a/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs new file mode 100644 index 00000000000..daf8dc82507 --- /dev/null +++ b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs @@ -0,0 +1,78 @@ +use serde::{Deserialize, Serialize}; + +use noirc_errors::Location; + +pub(crate) type CallStack = im::Vector; + +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub(crate) struct CallStackId(u32); + +impl CallStackId { + pub(crate) fn root() -> Self { + Self::new(0) + } + + fn new(id: usize) -> Self { + Self(id as u32) + } + + pub(crate) fn index(&self) -> usize { + self.0 as usize + } + + pub(crate) fn is_root(&self) -> bool { + self.0 == 0 + } + + /// Construct a CallStack from a CallStackId + pub(crate) fn get_call_stack(&self, locations: &[LocationNode]) -> CallStack { + let mut call_stack = im::Vector::new(); + let mut current_location = *self; + while let Some(parent) = locations[current_location.index()].parent { + call_stack.push_back(locations[current_location.index()].value); + current_location = parent; + } + call_stack + } + + /// Adds a location to the call stack + pub(crate) fn add_child( + &self, + location: Location, + locations: &mut Vec, + ) -> CallStackId { + if let Some(result) = locations[self.index()] + .children + .iter() + .rev() + .take(1000) + .find(|child| locations[child.index()].value == location) + { + return *result; + } + locations.push(LocationNode { parent: Some(*self), children: vec![], value: location }); + let new_location = CallStackId::new(locations.len() - 1); + locations[self.index()].children.push(new_location); + new_location + } + + /// Returns a new CallStackId which extends the current one with the provided call_stack. + pub(crate) fn extend( + &self, + call_stack: &CallStack, + locations: &mut Vec, + ) -> CallStackId { + let mut result = *self; + for location in call_stack { + result = result.add_child(*location, locations); + } + result + } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub(crate) struct LocationNode { + pub(crate) parent: Option, + pub(crate) children: Vec, + pub(crate) value: Location, +} diff --git a/compiler/noirc_evaluator/src/ssa/ir/cfg.rs b/compiler/noirc_evaluator/src/ssa/ir/cfg.rs index fc4d82c092d..788b1a7d302 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/cfg.rs @@ -128,7 +128,7 @@ impl ControlFlowGraph { #[cfg(test)] mod tests { use crate::ssa::ir::{ - dfg::CallStackId, instruction::TerminatorInstruction, map::Id, types::Type, + call_stack::CallStackId, instruction::TerminatorInstruction, map::Id, types::Type, }; use super::{super::function::Function, ControlFlowGraph}; diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index c5866a6366d..bd2f5e44e4d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -4,11 +4,11 @@ use crate::ssa::{function_builder::data_bus::DataBus, ir::instruction::SimplifyR use super::{ basic_block::{BasicBlock, BasicBlockId}, + call_stack::{CallStack, CallStackId, LocationNode}, function::FunctionId, instruction::{ Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction, }, - list::List, map::DenseMap, types::{NumericType, Type}, value::{Value, ValueId}, @@ -22,71 +22,6 @@ use serde::{Deserialize, Serialize}; use serde_with::serde_as; use serde_with::DisplayFromStr; -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub(crate) struct CallStackId(u32); - -impl CallStackId { - pub(crate) fn root() -> Self { - Self::new(0) - } - - fn new(id: usize) -> Self { - Self(id as u32) - } - - pub(crate) fn index(&self) -> usize { - self.0 as usize - } - - pub(crate) fn is_root(&self) -> bool { - self.0 == 0 - } - - /// Construct a CallStack from a CallStackId - fn get_call_stack(&self, locations: &[LocationNode]) -> CallStack { - let mut call_stack = List::new(); - let mut current_location = *self; - while let Some(parent) = locations[current_location.index()].parent { - call_stack.push_back(locations[current_location.index()].value); - current_location = parent; - } - call_stack - } - - /// Adds a location to the call stack - fn add_child(&self, location: Location, locations: &mut Vec) -> CallStackId { - if let Some(result) = locations[self.index()] - .children - .iter() - .rev() - .take(1000) - .find(|child| locations[child.index()].value == location) - { - return *result; - } - locations.push(LocationNode { parent: Some(*self), children: vec![], value: location }); - let new_location = CallStackId::new(locations.len() - 1); - locations[self.index()].children.push(new_location); - new_location - } - - /// Returns a new CallStackId which extends the current one with the provided call_stack. - fn extend(&self, call_stack: &CallStack, locations: &mut Vec) -> CallStackId { - let mut result = *self; - for location in call_stack { - result = result.add_child(*location, locations); - } - result - } -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -struct LocationNode { - parent: Option, - children: Vec, - value: Location, -} - /// The DataFlowGraph contains most of the actual data in a function including /// its blocks, instructions, and values. This struct is largely responsible for /// owning most data in a function and handing out Ids to this data that can be @@ -165,8 +100,6 @@ pub(crate) struct DataFlowGraph { pub(crate) data_bus: DataBus, } -pub(crate) type CallStack = List; - impl DataFlowGraph { /// Creates a new basic block with no parameters. /// After being created, the block is unreachable in the current function diff --git a/compiler/noirc_evaluator/src/ssa/ir/dom.rs b/compiler/noirc_evaluator/src/ssa/ir/dom.rs index 2b0bb200843..ff54bf3b6ed 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dom.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dom.rs @@ -272,8 +272,8 @@ mod tests { use crate::ssa::{ function_builder::FunctionBuilder, ir::{ - basic_block::BasicBlockId, dfg::CallStackId, dom::DominatorTree, function::Function, - instruction::TerminatorInstruction, map::Id, types::Type, + basic_block::BasicBlockId, call_stack::CallStackId, dom::DominatorTree, + function::Function, instruction::TerminatorInstruction, map::Id, types::Type, }, }; diff --git a/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs b/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs index 797feb6a9f5..dbe090c0fd6 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function_inserter.rs @@ -4,12 +4,12 @@ use crate::ssa::ir::types::Type; use super::{ basic_block::BasicBlockId, + call_stack::CallStackId, dfg::InsertInstructionResult, function::Function, instruction::{Instruction, InstructionId}, value::ValueId, }; -use crate::ssa::ir::dfg::CallStackId; use fxhash::FxHashMap as HashMap; /// The FunctionInserter can be used to help modify existing Functions diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index d6cb7968946..e91a69fd8f8 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -14,7 +14,8 @@ use crate::ssa::{ir::function::RuntimeType, opt::flatten_cfg::value_merger::Valu use super::{ basic_block::BasicBlockId, - dfg::{CallStackId, DataFlowGraph}, + call_stack::CallStackId, + dfg::DataFlowGraph, function::Function, map::Id, types::{NumericType, Type}, diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index bdb7d316a1a..fa7d314ff33 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -12,7 +12,8 @@ use num_bigint::BigUint; use crate::ssa::{ ir::{ basic_block::BasicBlockId, - dfg::{CallStackId, DataFlowGraph}, + call_stack::CallStackId, + dfg::DataFlowGraph, instruction::Intrinsic, map::Id, types::{NumericType, Type}, diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index 1b18baf71f4..ffacf6fe8b5 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use acvm::{acir::AcirField, BlackBoxFunctionSolver, BlackBoxResolutionError, FieldElement}; -use crate::ssa::ir::dfg::CallStackId; +use crate::ssa::ir::call_stack::CallStackId; use crate::ssa::ir::instruction::BlackBoxFunc; use crate::ssa::ir::types::NumericType; use crate::ssa::ir::{ diff --git a/compiler/noirc_evaluator/src/ssa/ir/list.rs b/compiler/noirc_evaluator/src/ssa/ir/list.rs deleted file mode 100644 index 9a84d304444..00000000000 --- a/compiler/noirc_evaluator/src/ssa/ir/list.rs +++ /dev/null @@ -1,187 +0,0 @@ -use serde::{Deserialize, Serialize}; -use std::sync::Arc; - -/// A shared linked list type intended to be cloned -#[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -pub struct List { - head: Arc>, - len: usize, -} - -#[derive(Default, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] -enum Node { - #[default] - Nil, - Cons(T, Arc>), -} - -impl Default for List { - fn default() -> Self { - List { head: Arc::new(Node::Nil), len: 0 } - } -} - -impl List { - pub fn new() -> Self { - Self::default() - } - - /// This is actually a push_front since we just create a new head for the - /// list. This is done so that the tail of the list can still be shared. - /// In the case of call stacks, the last node will be main, while the top - /// of the call stack will be the head of this list. - pub fn push_back(&mut self, value: T) { - self.len += 1; - self.head = Arc::new(Node::Cons(value, self.head.clone())); - } - - /// It is more efficient to iterate from the head of the list towards the tail. - /// For callstacks this means from the top of the call stack towards main. - fn iter_rev(&self) -> IterRev { - IterRev { head: &self.head, len: self.len } - } - - pub fn clear(&mut self) { - *self = Self::default(); - } - - pub fn append(&mut self, other: Self) - where - T: Copy + std::fmt::Debug, - { - let other = other.into_iter().collect::>(); - - for item in other { - self.push_back(item); - } - } - - pub fn len(&self) -> usize { - self.len - } - - pub fn is_empty(&self) -> bool { - self.len == 0 - } - - fn pop_front(&mut self) -> Option - where - T: Copy, - { - match self.head.as_ref() { - Node::Nil => None, - Node::Cons(value, rest) => { - let value = *value; - self.head = rest.clone(); - self.len -= 1; - Some(value) - } - } - } - - pub fn truncate(&mut self, len: usize) - where - T: Copy, - { - if self.len > len { - for _ in 0..self.len - len { - self.pop_front(); - } - } - } - - pub fn unit(item: T) -> Self { - let mut this = Self::default(); - this.push_back(item); - this - } - - pub fn back(&self) -> Option<&T> { - match self.head.as_ref() { - Node::Nil => None, - Node::Cons(item, _) => Some(item), - } - } -} - -pub struct IterRev<'a, T> { - head: &'a Node, - len: usize, -} - -impl IntoIterator for List -where - T: Copy + std::fmt::Debug, -{ - type Item = T; - - type IntoIter = std::iter::Rev>; - - fn into_iter(self) -> Self::IntoIter { - let items: Vec<_> = self.iter_rev().copied().collect(); - items.into_iter().rev() - } -} - -impl<'a, T> IntoIterator for &'a List { - type Item = &'a T; - - type IntoIter = std::iter::Rev< as IntoIterator>::IntoIter>; - - fn into_iter(self) -> Self::IntoIter { - let items: Vec<_> = self.iter_rev().collect(); - items.into_iter().rev() - } -} - -impl<'a, T> Iterator for IterRev<'a, T> { - type Item = &'a T; - - fn next(&mut self) -> Option { - match self.head { - Node::Nil => None, - Node::Cons(value, rest) => { - self.head = rest; - Some(value) - } - } - } - - fn size_hint(&self) -> (usize, Option) { - (0, Some(self.len)) - } -} - -impl<'a, T> ExactSizeIterator for IterRev<'a, T> {} - -impl std::fmt::Debug for List -where - T: std::fmt::Debug, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "[")?; - for (i, item) in self.iter_rev().enumerate() { - if i != 0 { - write!(f, ", ")?; - } - write!(f, "{item:?}")?; - } - write!(f, "]") - } -} - -impl std::fmt::Display for List -where - T: std::fmt::Display, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "[")?; - for (i, item) in self.iter_rev().enumerate() { - if i != 0 { - write!(f, ", ")?; - } - write!(f, "{item}")?; - } - write!(f, "]") - } -} diff --git a/compiler/noirc_evaluator/src/ssa/ir/mod.rs b/compiler/noirc_evaluator/src/ssa/ir/mod.rs index 89ba22e8b79..686606452fb 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/mod.rs @@ -1,11 +1,11 @@ pub(crate) mod basic_block; +pub(crate) mod call_stack; pub(crate) mod cfg; pub(crate) mod dfg; pub(crate) mod dom; pub(crate) mod function; pub(crate) mod function_inserter; pub(crate) mod instruction; -pub mod list; pub(crate) mod map; pub(crate) mod post_order; pub(crate) mod printer; diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index 66a0d8a349e..4db56cdfebc 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -6,7 +6,8 @@ use rayon::iter::{IntoParallelRefMutIterator, ParallelIterator}; use crate::ssa::{ ir::{ basic_block::{BasicBlock, BasicBlockId}, - dfg::{CallStackId, DataFlowGraph}, + call_stack::CallStackId, + dfg::DataFlowGraph, function::Function, instruction::{BinaryOp, Instruction, InstructionId, Intrinsic}, post_order::PostOrder, diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs index 9deccc7c723..9d286825a2e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs @@ -139,8 +139,9 @@ use iter_extended::vecmap; use crate::ssa::{ ir::{ basic_block::BasicBlockId, + call_stack::CallStackId, cfg::ControlFlowGraph, - dfg::{CallStackId, InsertInstructionResult}, + dfg::InsertInstructionResult, function::{Function, FunctionId, RuntimeType}, function_inserter::FunctionInserter, instruction::{BinaryOp, Instruction, InstructionId, Intrinsic, TerminatorInstruction}, diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs index 2f5eddd3d62..df351d6c0cd 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg/value_merger.rs @@ -3,7 +3,8 @@ use fxhash::{FxHashMap as HashMap, FxHashSet}; use crate::ssa::ir::{ basic_block::BasicBlockId, - dfg::{CallStackId, DataFlowGraph, InsertInstructionResult}, + call_stack::CallStackId, + dfg::{DataFlowGraph, InsertInstructionResult}, instruction::{BinaryOp, Instruction}, types::{NumericType, Type}, value::{Value, ValueId}, diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index e61a6a9eb29..0f638a8964f 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -11,7 +11,8 @@ use crate::ssa::{ function_builder::FunctionBuilder, ir::{ basic_block::BasicBlockId, - dfg::{CallStackId, InsertInstructionResult}, + call_stack::CallStackId, + dfg::InsertInstructionResult, function::{Function, FunctionId, RuntimeType}, instruction::{Instruction, InstructionId, TerminatorInstruction}, value::{Value, ValueId}, diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index f0e22b73ebe..4c5189d8c91 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -5,7 +5,8 @@ use acvm::{acir::AcirField, FieldElement}; use crate::ssa::{ ir::{ basic_block::BasicBlockId, - dfg::{CallStackId, InsertInstructionResult}, + call_stack::CallStackId, + dfg::InsertInstructionResult, function::{Function, RuntimeType}, instruction::{Binary, BinaryOp, Endian, Instruction, InstructionId, Intrinsic}, types::{NumericType, Type}, diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index b6622f3b8da..5377b0c9695 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -27,8 +27,9 @@ use crate::{ ssa::{ ir::{ basic_block::BasicBlockId, + call_stack::{CallStack, CallStackId}, cfg::ControlFlowGraph, - dfg::{CallStack, CallStackId, DataFlowGraph}, + dfg::DataFlowGraph, dom::DominatorTree, function::Function, function_inserter::{ArrayCache, FunctionInserter}, From 7d42406d35ddea48a3061454f9d01bb07c2ce9b8 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 13 Dec 2024 17:21:46 +0000 Subject: [PATCH 10/13] fix unit test --- compiler/noirc_evaluator/src/acir/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 91d46595cef..798ecad9f5c 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -2905,7 +2905,7 @@ mod test { ssa::{ function_builder::FunctionBuilder, ir::{ - dfg::CallStack, + call_stack::CallStack, function::FunctionId, instruction::BinaryOp, map::Id, From 10cdd9226062fc6c21372c6d3ee557c67df98d94 Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 13 Dec 2024 18:49:57 +0000 Subject: [PATCH 11/13] refactor call_stack --- compiler/noirc_evaluator/src/acir/mod.rs | 5 +- .../src/ssa/function_builder/mod.rs | 5 +- .../noirc_evaluator/src/ssa/ir/call_stack.rs | 96 +++++++++++++------ compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 59 ++---------- .../noirc_evaluator/src/ssa/ir/function.rs | 2 +- .../noirc_evaluator/src/ssa/opt/inlining.rs | 9 +- .../src/ssa/opt/normalize_value_ids.rs | 6 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 8 +- 8 files changed, 97 insertions(+), 93 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 798ecad9f5c..e7b011b6d7b 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -1842,7 +1842,7 @@ impl<'a> Context<'a> { } } - let call_stack = dfg.get_call_stack(call_stack); + let call_stack = dfg.call_stack_data.get_call_stack(call_stack); let warnings = if has_constant_return { vec![SsaReport::Warning(InternalWarning::ReturnConstant { call_stack })] } else { @@ -2934,7 +2934,8 @@ mod test { // Set a call stack for testing whether `brillig_locations` in the `GeneratedAcir` was accurately set. let mut stack = CallStack::unit(Location::dummy()); stack.push_back(Location::dummy()); - let call_stack = builder.current_function.dfg.get_or_insert_locations(stack); + let call_stack = + builder.current_function.dfg.call_stack_data.get_or_insert_locations(stack); builder.set_call_stack(call_stack); let foo_v0 = builder.add_parameter(Type::field()); diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 0c58f27447e..855034cedd2 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -85,7 +85,8 @@ impl FunctionBuilder { self.current_block = new_function.entry_block(); let old_function = std::mem::replace(&mut self.current_function, new_function); // Copy the call stack to the new function - self.call_stack = self.current_function.dfg.get_or_insert_locations(call_stack); + self.call_stack = + self.current_function.dfg.call_stack_data.get_or_insert_locations(call_stack); self.finished_functions.push(old_function); } @@ -199,7 +200,7 @@ impl FunctionBuilder { } pub(crate) fn set_location(&mut self, location: Location) -> &mut FunctionBuilder { - self.call_stack = self.current_function.dfg.add_location_to_root(location); + self.call_stack = self.current_function.dfg.call_stack_data.add_location_to_root(location); self } diff --git a/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs index daf8dc82507..cadfb5d8e23 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs @@ -23,56 +23,92 @@ impl CallStackId { pub(crate) fn is_root(&self) -> bool { self.0 == 0 } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub(crate) struct LocationNode { + pub(crate) parent: Option, + pub(crate) children: Vec, + pub(crate) value: Location, +} +#[derive(Debug, Default, Clone, Serialize, Deserialize)] +pub(crate) struct CallStackHelper { + locations: Vec, +} + +impl CallStackHelper { /// Construct a CallStack from a CallStackId - pub(crate) fn get_call_stack(&self, locations: &[LocationNode]) -> CallStack { - let mut call_stack = im::Vector::new(); - let mut current_location = *self; - while let Some(parent) = locations[current_location.index()].parent { - call_stack.push_back(locations[current_location.index()].value); - current_location = parent; + pub(crate) fn get_call_stack(&self, mut call_stack: CallStackId) -> CallStack { + let mut result = im::Vector::new(); + while let Some(parent) = self.locations[call_stack.index()].parent { + result.push_back(self.locations[call_stack.index()].value); + call_stack = parent; + } + result + } + + /// Returns a new CallStackId which extends the call_stack with the provided call_stack. + pub(crate) fn extend_call_stack( + &mut self, + mut call_stack: CallStackId, + locations: &CallStack, + ) -> CallStackId { + for location in locations { + call_stack = self.add_child(call_stack, *location); } call_stack } /// Adds a location to the call stack - pub(crate) fn add_child( - &self, - location: Location, - locations: &mut Vec, - ) -> CallStackId { - if let Some(result) = locations[self.index()] + pub(crate) fn add_child(&mut self, call_stack: CallStackId, location: Location) -> CallStackId { + if let Some(result) = self.locations[call_stack.index()] .children .iter() .rev() .take(1000) - .find(|child| locations[child.index()].value == location) + .find(|child| self.locations[child.index()].value == location) { return *result; } - locations.push(LocationNode { parent: Some(*self), children: vec![], value: location }); - let new_location = CallStackId::new(locations.len() - 1); - locations[self.index()].children.push(new_location); + self.locations.push(LocationNode { + parent: Some(call_stack), + children: vec![], + value: location, + }); + let new_location = CallStackId::new(self.locations.len() - 1); + self.locations[call_stack.index()].children.push(new_location); new_location } - /// Returns a new CallStackId which extends the current one with the provided call_stack. - pub(crate) fn extend( + /// Retrieve the CallStackId corresponding to call_stack with the last 'len' locations removed. + pub(crate) fn unwind_call_stack( &self, - call_stack: &CallStack, - locations: &mut Vec, + mut call_stack: CallStackId, + mut len: usize, ) -> CallStackId { - let mut result = *self; - for location in call_stack { - result = result.add_child(*location, locations); + while len > 0 { + if let Some(parent) = self.locations[call_stack.index()].parent { + len -= 1; + call_stack = parent; + } else { + break; + } } - result + call_stack } -} -#[derive(Debug, Clone, Serialize, Deserialize)] -pub(crate) struct LocationNode { - pub(crate) parent: Option, - pub(crate) children: Vec, - pub(crate) value: Location, + pub(crate) fn add_location_to_root(&mut self, location: Location) -> CallStackId { + if self.locations.is_empty() { + self.locations.push(LocationNode { parent: None, children: vec![], value: location }); + CallStackId::root() + } else { + self.add_child(CallStackId::root(), location) + } + } + + /// Get (or create) a CallStackId corresponding to the given locations + pub(crate) fn get_or_insert_locations(&mut self, locations: CallStack) -> CallStackId { + self.extend_call_stack(CallStackId::root(), &locations) + } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index bd2f5e44e4d..72ba369f98c 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -4,7 +4,7 @@ use crate::ssa::{function_builder::data_bus::DataBus, ir::instruction::SimplifyR use super::{ basic_block::{BasicBlock, BasicBlockId}, - call_stack::{CallStack, CallStackId, LocationNode}, + call_stack::{CallStack, CallStackHelper, CallStackId}, function::FunctionId, instruction::{ Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction, @@ -94,7 +94,7 @@ pub(crate) struct DataFlowGraph { #[serde(skip)] locations: HashMap, - location_array: Vec, + pub(crate) call_stack_data: CallStackHelper, #[serde(skip)] pub(crate) data_bus: DataBus, @@ -489,7 +489,7 @@ impl DataFlowGraph { pub(crate) fn get_instruction_call_stack(&self, instruction: InstructionId) -> CallStack { let call_stack = self.get_instruction_call_stack_id(instruction); - self.get_call_stack(call_stack) + self.call_stack_data.get_call_stack(call_stack) } pub(crate) fn get_instruction_call_stack_id(&self, instruction: InstructionId) -> CallStackId { @@ -502,29 +502,11 @@ impl DataFlowGraph { location: Location, ) { let call_stack = self.locations.entry(instruction).or_default(); - call_stack.add_child(location, &mut self.location_array); + *call_stack = self.call_stack_data.add_child(*call_stack, location); } - pub(crate) fn add_location_to_root(&mut self, location: Location) -> CallStackId { - if self.location_array.is_empty() { - self.location_array.push(LocationNode { - parent: None, - children: vec![], - value: location, - }); - CallStackId::root() - } else { - CallStackId::root().add_child(location, &mut self.location_array) - } - } - - /// Get (or create) a CallStackId corresponding to the given locations - pub(crate) fn get_or_insert_locations(&mut self, locations: CallStack) -> CallStackId { - let mut result = CallStackId::root(); - for location in locations { - result = result.add_child(location, &mut self.location_array); - } - result + pub(crate) fn get_call_stack(&self, call_stack: CallStackId) -> CallStack { + self.call_stack_data.get_call_stack(call_stack) } pub(crate) fn get_value_call_stack(&self, value: ValueId) -> CallStack { @@ -543,35 +525,6 @@ impl DataFlowGraph { } } - pub(crate) fn get_call_stack(&self, call_stack: CallStackId) -> CallStack { - call_stack.get_call_stack(&self.location_array) - } - - pub(crate) fn extend_call_stack( - &mut self, - call_stack: CallStackId, - locations: &CallStack, - ) -> CallStackId { - call_stack.extend(locations, &mut self.location_array) - } - - // Retrieve the CallStackId corresponding to call_stack with the last 'len' locations removed. - pub(crate) fn unwind_call_stack( - &self, - mut call_stack: CallStackId, - mut len: usize, - ) -> CallStackId { - while len > 0 { - if let Some(parent) = self.location_array[call_stack.index()].parent { - len -= 1; - call_stack = parent; - } else { - break; - } - } - call_stack - } - /// True if the given ValueId refers to a (recursively) constant value pub(crate) fn is_constant(&self, argument: ValueId) -> bool { match &self[self.resolve(argument)] { diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 189331a5ac2..49217c7a8b8 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -87,7 +87,7 @@ impl Function { pub(crate) fn new(name: String, id: FunctionId) -> Self { let mut dfg = DataFlowGraph::default(); // Adds root node for the location tree - dfg.add_location_to_root(Location::dummy()); + dfg.call_stack_data.add_location_to_root(Location::dummy()); let entry_block = dfg.make_block(); Self { name, id, entry_block, dfg, runtime: RuntimeType::Acir(InlineType::default()) } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 0f638a8964f..11201fc8f85 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -668,6 +668,7 @@ impl<'function> PerFunctionContext<'function> { .builder .current_function .dfg + .call_stack_data .extend_call_stack(self.context.call_stack, &call_stack); self.context.call_stack = new_call_stack; @@ -677,6 +678,7 @@ impl<'function> PerFunctionContext<'function> { .builder .current_function .dfg + .call_stack_data .unwind_call_stack(self.context.call_stack, call_stack_len); let new_results = InsertInstructionResult::Results(call_id, &new_results); @@ -695,6 +697,7 @@ impl<'function> PerFunctionContext<'function> { .builder .current_function .dfg + .call_stack_data .extend_call_stack(call_stack, &source_call_stack); let results = self.source_function.dfg.instruction_results(id); let results = vecmap(results, |id| self.source_function.dfg.resolve(*id)); @@ -758,6 +761,7 @@ impl<'function> PerFunctionContext<'function> { .builder .current_function .dfg + .call_stack_data .extend_call_stack(self.context.call_stack, &call_stack); self.context @@ -779,6 +783,7 @@ impl<'function> PerFunctionContext<'function> { .builder .current_function .dfg + .call_stack_data .extend_call_stack(self.context.call_stack, &call_stack); // See if the value of the condition is known, and if so only inline the reachable @@ -816,12 +821,14 @@ impl<'function> PerFunctionContext<'function> { let block_id = self.context.builder.current_block(); if self.inlining_entry { - let call_stack = self.source_function.dfg.get_call_stack(*call_stack); + let call_stack = + self.source_function.dfg.call_stack_data.get_call_stack(*call_stack); let new_call_stack = self .context .builder .current_function .dfg + .call_stack_data .extend_call_stack(self.context.call_stack, &call_stack); self.context diff --git a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs index ea640042ed3..f8ad5eafbf7 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/normalize_value_ids.rs @@ -88,7 +88,8 @@ impl Context { let call_stack = old_function.dfg.get_instruction_call_stack_id(old_instruction_id); let locations = old_function.dfg.get_call_stack(call_stack); - let new_call_stack = new_function.dfg.get_or_insert_locations(locations); + let new_call_stack = + new_function.dfg.call_stack_data.get_or_insert_locations(locations); let old_results = old_function.dfg.instruction_results(old_instruction_id); let ctrl_typevars = instruction @@ -116,7 +117,8 @@ impl Context { .map_values(|value| self.new_ids.map_value(new_function, old_function, value)); terminator.mutate_blocks(|old_block| self.new_ids.blocks[&old_block]); let locations = old_function.dfg.get_call_stack(terminator.call_stack()); - let new_call_stack = new_function.dfg.get_or_insert_locations(locations); + let new_call_stack = + new_function.dfg.call_stack_data.get_or_insert_locations(locations); terminator.set_call_stack(new_call_stack); new_function.dfg.set_block_terminator(new_block_id, terminator); } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 07457e0e808..d3821158b80 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -91,8 +91,12 @@ pub(crate) fn generate_ssa( None, ); } - let return_call_stack = - function_context.builder.current_function.dfg.add_location_to_root(return_location); + let return_call_stack = function_context + .builder + .current_function + .dfg + .call_stack_data + .add_location_to_root(return_location); let return_instruction = function_context.builder.current_function.dfg[block].unwrap_terminator_mut(); From 1299d7b0e2281e10c94f8a4fe63558e6a246e794 Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 16 Dec 2024 10:02:11 +0000 Subject: [PATCH 12/13] Code review --- compiler/noirc_evaluator/src/ssa/ir/call_stack.rs | 11 ++++++++++- compiler/noirc_evaluator/src/ssa/ir/function.rs | 3 --- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs index cadfb5d8e23..25c0b80f73c 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs @@ -32,11 +32,20 @@ pub(crate) struct LocationNode { pub(crate) value: Location, } -#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct CallStackHelper { locations: Vec, } +impl Default for CallStackHelper { + /// Generates a new helper, with an empty location tree + fn default() -> Self { + let mut result = CallStackHelper { locations: Vec::new() }; + result.add_location_to_root(Location::dummy()); + result + } +} + impl CallStackHelper { /// Construct a CallStack from a CallStackId pub(crate) fn get_call_stack(&self, mut call_stack: CallStackId) -> CallStack { diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 49217c7a8b8..6413107c04a 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -10,7 +10,6 @@ use super::instruction::TerminatorInstruction; use super::map::Id; use super::types::Type; use super::value::ValueId; -use noirc_errors::Location; #[derive(Clone, Copy, PartialEq, Eq, Debug, Hash, Serialize, Deserialize)] pub(crate) enum RuntimeType { @@ -86,8 +85,6 @@ impl Function { /// Note that any parameters or attributes of the function must be manually added later. pub(crate) fn new(name: String, id: FunctionId) -> Self { let mut dfg = DataFlowGraph::default(); - // Adds root node for the location tree - dfg.call_stack_data.add_location_to_root(Location::dummy()); let entry_block = dfg.make_block(); Self { name, id, entry_block, dfg, runtime: RuntimeType::Acir(InlineType::default()) } } From 70aab82eaaeca4b62322d93fdb0cf97c16ec5966 Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 16 Dec 2024 10:54:47 +0000 Subject: [PATCH 13/13] code review: trying with a hashmap using location hash as key --- .../noirc_evaluator/src/ssa/ir/call_stack.rs | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs index 25c0b80f73c..113609f4a11 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/call_stack.rs @@ -1,3 +1,4 @@ +use fxhash::FxHashMap; use serde::{Deserialize, Serialize}; use noirc_errors::Location; @@ -29,9 +30,16 @@ impl CallStackId { pub(crate) struct LocationNode { pub(crate) parent: Option, pub(crate) children: Vec, + pub(crate) children_hash: FxHashMap, pub(crate) value: Location, } +impl LocationNode { + pub(crate) fn new(parent: Option, value: Location) -> Self { + LocationNode { parent, children: Vec::new(), children_hash: FxHashMap::default(), value } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct CallStackHelper { locations: Vec, @@ -71,23 +79,20 @@ impl CallStackHelper { /// Adds a location to the call stack pub(crate) fn add_child(&mut self, call_stack: CallStackId, location: Location) -> CallStackId { - if let Some(result) = self.locations[call_stack.index()] - .children - .iter() - .rev() - .take(1000) - .find(|child| self.locations[child.index()].value == location) - { - return *result; + let key = fxhash::hash64(&location); + if let Some(result) = self.locations[call_stack.index()].children_hash.get(&key) { + if self.locations[result.index()].value == location { + return *result; + } } - self.locations.push(LocationNode { - parent: Some(call_stack), - children: vec![], - value: location, - }); - let new_location = CallStackId::new(self.locations.len() - 1); - self.locations[call_stack.index()].children.push(new_location); - new_location + let new_location = LocationNode::new(Some(call_stack), location); + let key = fxhash::hash64(&new_location.value); + self.locations.push(new_location); + let new_location_id = CallStackId::new(self.locations.len() - 1); + + self.locations[call_stack.index()].children.push(new_location_id); + self.locations[call_stack.index()].children_hash.insert(key, new_location_id); + new_location_id } /// Retrieve the CallStackId corresponding to call_stack with the last 'len' locations removed. @@ -109,7 +114,7 @@ impl CallStackHelper { pub(crate) fn add_location_to_root(&mut self, location: Location) -> CallStackId { if self.locations.is_empty() { - self.locations.push(LocationNode { parent: None, children: vec![], value: location }); + self.locations.push(LocationNode::new(None, location)); CallStackId::root() } else { self.add_child(CallStackId::root(), location)