From f719307bfb95193e30c0f8f8d81ab210b30f3780 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 3 May 2023 11:07:46 -0500 Subject: [PATCH 1/5] Start inlining pass --- crates/noirc_evaluator/src/ssa_refactor.rs | 1 + .../src/ssa_refactor/ir/basic_block.rs | 13 +- .../src/ssa_refactor/ir/dfg.rs | 44 ++++--- .../src/ssa_refactor/ir/map.rs | 12 ++ .../src/ssa_refactor/opt/inlining.rs | 124 ++++++++++++++++++ .../src/ssa_refactor/opt/mod.rs | 6 + .../src/ssa_refactor/ssa_gen/program.rs | 22 +++- 7 files changed, 196 insertions(+), 26 deletions(-) create mode 100644 crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs create mode 100644 crates/noirc_evaluator/src/ssa_refactor/opt/mod.rs diff --git a/crates/noirc_evaluator/src/ssa_refactor.rs b/crates/noirc_evaluator/src/ssa_refactor.rs index fc45071e57..a33393866d 100644 --- a/crates/noirc_evaluator/src/ssa_refactor.rs +++ b/crates/noirc_evaluator/src/ssa_refactor.rs @@ -8,5 +8,6 @@ #![allow(dead_code)] mod ir; +mod opt; mod ssa_builder; pub mod ssa_gen; diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs index 8a3f74c4a6..8bf1711902 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs @@ -29,10 +29,10 @@ pub(crate) struct BasicBlock { pub(crate) type BasicBlockId = Id; impl BasicBlock { - /// Create a new BasicBlock with the given parameters. + /// Create a new BasicBlock with the given instructions. /// Parameters can also be added later via BasicBlock::add_parameter - pub(crate) fn new(parameters: Vec) -> Self { - Self { parameters, instructions: Vec::new(), terminator: None } + pub(crate) fn new(instructions: Vec) -> Self { + Self { parameters: Vec::new(), instructions, terminator: None } } /// Returns the parameters of this block @@ -57,6 +57,11 @@ impl BasicBlock { &self.instructions } + /// Retrieve a mutable reference to all instructions in this block. + pub(crate) fn instructions_mut(&mut self) -> &mut Vec { + &mut self.instructions + } + /// Sets the terminator instruction of this block. /// /// A properly-constructed block will always terminate with a TerminatorInstruction - @@ -90,7 +95,7 @@ impl BasicBlock { /// Removes the given instruction from this block if present or panics otherwise. pub(crate) fn remove_instruction(&mut self, instruction: InstructionId) { let index = - self.instructions.iter().position(|id| *id == instruction).unwrap_or_else(|| { + self.instructions.iter().rev().position(|id| *id == instruction).unwrap_or_else(|| { panic!("remove_instruction: No such instruction {instruction:?} in block") }); self.instructions.remove(index); diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 67569c6a4c..2ad6af991c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -13,7 +13,6 @@ use super::{ }; use acvm::FieldElement; -use iter_extended::vecmap; /// The DataFlowGraph contains most of the actual data in a function including /// its blocks, instructions, and values. This struct is largely responsible for @@ -69,22 +68,6 @@ impl DataFlowGraph { self.blocks.insert(BasicBlock::new(Vec::new())) } - /// Creates a new basic block with the given parameters. - /// After being created, the block is unreachable in the current function - /// until another block is made to jump to it. - pub(crate) fn make_block_with_parameters( - &mut self, - parameter_types: impl Iterator, - ) -> BasicBlockId { - self.blocks.insert_with_id(|entry_block| { - let parameters = vecmap(parameter_types.enumerate(), |(position, typ)| { - self.values.insert(Value::Param { block: entry_block, position, typ }) - }); - - BasicBlock::new(parameters) - }) - } - /// Get an iterator over references to each basic block within the dfg, paired with the basic /// block's id. /// @@ -279,6 +262,33 @@ impl DataFlowGraph { ) { self.blocks[block].set_terminator(terminator); } + + /// Splits the given block in two at the given instruction, returning the Id of the new block. + /// This will remove the given instruction and place every instruction after it into a new block + /// with the same terminator as the old block. The old block is modified to stop + /// before the instruction to remove and to unconditionally branch to the new block. + /// This function is useful during function inlining to remove the call instruction + /// while opening a spot at the end of the current block to insert instructions into. + /// + /// Example (before): + /// block1: a; b; c; d; e; jmp block5 + /// + /// After self.split_block_at(block1, c): + /// block1: a; b; jmp block2 + /// block2: d; e; jmp block5 + pub(crate) fn split_block_at(&mut self, block: BasicBlockId, instruction_to_remove: InstructionId) -> BasicBlockId { + let split_block = &mut self.blocks[block]; + + let mut instructions = split_block.instructions().iter(); + let index = instructions.position(|id| *id == instruction_to_remove).unwrap_or_else(|| { + panic!("No instruction found with id {instruction_to_remove:?} in block {block:?}") + }); + + let instructions = split_block.instructions_mut().drain(index..).collect(); + split_block.remove_instruction(instruction_to_remove); + + self.blocks.insert(BasicBlock::new(instructions)) + } } impl std::ops::Index for DataFlowGraph { diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs index 14ea521359..eee1b593ed 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs @@ -45,6 +45,18 @@ impl std::hash::Hash for Id { } } +impl PartialOrd for Id { + fn partial_cmp(&self, other: &Self) -> Option { + self.index.partial_cmp(&other.index) + } +} + +impl Ord for Id { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.index.cmp(&other.index) + } +} + impl Eq for Id {} impl PartialEq for Id { diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs new file mode 100644 index 0000000000..9ccf1e7994 --- /dev/null +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -0,0 +1,124 @@ +use std::collections::{HashMap, HashSet}; + +use iter_extended::vecmap; + +use crate::ssa_refactor::{ssa_gen::Ssa, ir::{instruction::Instruction, value::{ValueId, Value}, dfg::DataFlowGraph, function::FunctionId, basic_block::BasicBlockId}}; + +/// An arbitrary limit to the maximum number of recursive call +/// frames at any point in time. +const RECURSION_LIMIT: u32 = 1000; + +impl Ssa { + /// Inline all functions within the IR. + /// + /// In the case of recursive functions, this will attempt + /// to recursively inline until the RECURSION_LIMIT is reached. + /// + /// Functions are recursively inlined into main until either we finish + /// inlining all functions or we encounter a function whose function id is not known. + /// When the later happens, the call instruction is kept in addition to the function + /// it refers to. The function it refers to is kept unmodified without any inlining + /// changes. This is because if the function's id later becomes known by a later + /// pass, we would need to re-run all of inlining anyway to inline it, so we might + /// as well save the work for later instead of performing it twice. + pub(crate) fn inline_functions(&mut self) { + let main_function = self.main(); + let mut context = InlineContext::new(main_function.entry_block(), main_function.id()); + + let blocks = vecmap(main_function.dfg.basic_blocks_iter(), |(id, _)| id); + + for block in blocks { + let instructions = main_function.dfg[block].instructions(); + + let mut new_instructions = Vec::with_capacity(instructions.len()); + + for (index, instruction) in instructions.iter().copied().enumerate() { + match &main_function.dfg[instruction] { + Instruction::Call { func, arguments } => { + match context.get_function(*func, &main_function.dfg) { + Some(id) => { + main_function.dfg.split_block_at(block, instruction); + context.inline_function(self, id, arguments) + } + None => new_instructions.push(instruction), + } + }, + _ => new_instructions.push(instruction), + } + } + } + } +} + +struct InlineContext { + recursion_level: u32, + argument_values: Vec>, + current_block: BasicBlockId, + visited_blocks: HashSet, + functions_to_keep: HashSet, +} + +impl InlineContext { + /// Create a new context object for the function inlining pass. + /// This starts off with an empty mapping of instructions for main's parameters. + fn new(current_block: BasicBlockId, main: FunctionId) -> InlineContext { + let mut visited_blocks = HashSet::new(); + visited_blocks.insert(current_block); + + let mut functions_to_keep = HashSet::new(); + functions_to_keep.insert(main); + + Self { + recursion_level: 0, + argument_values: vec![HashMap::new()], + current_block, + visited_blocks, + functions_to_keep, + } + } + + fn current_function_arguments(&self) -> &HashMap { + self.argument_values.last() + .expect("Expected there to always be argument values for the current function being inlined") + } + + fn get_function(&self, mut id: ValueId, dfg: &DataFlowGraph) -> Option { + if let Some(new_id) = self.current_function_arguments().get(&id) { + id = *new_id; + } + + match dfg[id] { + Value::Function(id) => Some(id), + _ => None, + } + } + + fn inline_function(&mut self, ssa: &Ssa, id: FunctionId, arguments: &[ValueId]) { + let target_function = &ssa.functions[&id]; + let current_block = target_function.entry_block(); + + let parameters = target_function.dfg.block_parameters(current_block); + assert_eq!(parameters.len(), arguments.len()); + + let argument_map = parameters.iter().copied().zip(arguments.iter().copied()).collect(); + self.argument_values.push(argument_map); + + let instructions = target_function.dfg[current_block].instructions(); + + let mut new_instructions = Vec::with_capacity(instructions.len()); + + for id in instructions { + match &target_function.dfg[*id] { + Instruction::Call { func, arguments } => { + match self.get_function(*func, &target_function.dfg) { + Some(id) => self.inline_function(ssa, id, arguments), + None => new_instructions.push(*id), + } + }, + _ => new_instructions.push(*id), + } + } + + self.argument_values.pop(); + } +} diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/mod.rs new file mode 100644 index 0000000000..46ca7d443b --- /dev/null +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/mod.rs @@ -0,0 +1,6 @@ +//! This folder contains each optimization pass for the SSA IR. +//! +//! Each pass is generally expected to mutate the SSA IR into a gradually +//! simpler form until the IR only has a single function remaining with 1 block within it. +//! Generally, these passes are also expected to minimize the final amount of instructions. +mod inlining; diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs index 99d4945621..22bff853f0 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs @@ -1,22 +1,34 @@ -use std::fmt::Display; +use std::{collections::BTreeMap, fmt::Display}; -use crate::ssa_refactor::ir::function::Function; +use iter_extended::btree_map; + +use crate::ssa_refactor::ir::function::{Function, FunctionId}; /// Contains the entire SSA representation of the program. +/// +/// It is expected that the main function is always the first +/// function in the functions vector. pub struct Ssa { - functions: Vec, + pub functions: BTreeMap, } impl Ssa { /// Create a new Ssa object from the given SSA functions pub fn new(functions: Vec) -> Self { - Self { functions } + Self { functions: btree_map(functions, |f| (f.id(), f)) } + } + + pub fn main(&mut self) -> &mut Function { + self.functions + .first_entry() + .expect("Expected there to be at least 1 SSA function") + .into_mut() } } impl Display for Ssa { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for function in &self.functions { + for (_, function) in &self.functions { writeln!(f, "{function}")?; } Ok(()) From 658b7649b3681c65dbf3d47aff9463c8b3d547b9 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 3 May 2023 14:47:33 -0500 Subject: [PATCH 2/5] Get most of pass working --- .../src/ssa_refactor/ir/instruction.rs | 39 ++- .../src/ssa_refactor/opt/inlining.rs | 266 +++++++++++++----- .../src/ssa_refactor/ssa_builder/mod.rs | 23 +- .../src/ssa_refactor/ssa_gen/program.rs | 2 +- 4 files changed, 259 insertions(+), 71 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index 66f8b1e3b1..812d12b23a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -1,4 +1,5 @@ use acvm::acir::BlackBoxFunc; +use iter_extended::vecmap; use super::{basic_block::BasicBlockId, map::Id, types::Type, value::ValueId}; @@ -114,6 +115,42 @@ impl Instruction { Instruction::Load { .. } | Instruction::Call { .. } => InstructionResultType::Unknown, } } + + /// True if this instruction requires specifying the control type variables when + /// inserting this instruction into a DataFlowGraph. + pub(crate) fn requires_ctrl_typevars(&self) -> bool { + matches!(self.result_type(), InstructionResultType::Unknown) + } + + /// Maps each ValueId inside this instruction to a new ValueId, returning the new instruction. + /// Note that the returned instruction is fresh and will not have an assigned InstructionId + /// until it is manually inserted in a DataFlowGraph later. + pub(crate) fn map_values(&self, mut f: impl FnMut(ValueId) -> ValueId) -> Instruction { + match self { + Instruction::Binary(binary) => Instruction::Binary(Binary { + lhs: f(binary.lhs), + rhs: f(binary.rhs), + operator: binary.operator, + }), + Instruction::Cast(value, typ) => Instruction::Cast(f(*value), *typ), + Instruction::Not(value) => Instruction::Not(f(*value)), + Instruction::Truncate { value, bit_size, max_bit_size } => Instruction::Truncate { + value: f(*value), + bit_size: *bit_size, + max_bit_size: *max_bit_size, + }, + Instruction::Constrain(value) => Instruction::Constrain(f(*value)), + Instruction::Call { func, arguments } => Instruction::Call { + func: f(*func), + arguments: vecmap(arguments.iter().copied(), f), + }, + Instruction::Allocate { size } => Instruction::Allocate { size: *size }, + Instruction::Load { address } => Instruction::Load { address: f(*address) }, + Instruction::Store { address, value } => { + Instruction::Store { address: f(*address), value: f(*value) } + } + } + } } /// The possible return values for Instruction::return_types @@ -191,7 +228,7 @@ impl Binary { /// All binary operators are also only for numeric types. To implement /// e.g. equality for a compound type like a struct, one must add a /// separate Eq operation for each field and combine them later with And. -#[derive(Debug, PartialEq, Eq, Hash, Clone)] +#[derive(Debug, PartialEq, Eq, Hash, Copy, Clone)] pub(crate) enum BinaryOp { /// Addition of lhs + rhs. Add, diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index 9ccf1e7994..3bfcf934ad 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -2,7 +2,16 @@ use std::collections::{HashMap, HashSet}; use iter_extended::vecmap; -use crate::ssa_refactor::{ssa_gen::Ssa, ir::{instruction::Instruction, value::{ValueId, Value}, dfg::DataFlowGraph, function::FunctionId, basic_block::BasicBlockId}}; +use crate::ssa_refactor::{ + ir::{ + basic_block::BasicBlockId, + function::{Function, FunctionId}, + instruction::{Instruction, InstructionId, TerminatorInstruction}, + value::{Value, ValueId}, + }, + ssa_builder::FunctionBuilder, + ssa_gen::Ssa, +}; /// An arbitrary limit to the maximum number of recursive call /// frames at any point in time. @@ -23,102 +32,229 @@ impl Ssa { /// as well save the work for later instead of performing it twice. pub(crate) fn inline_functions(&mut self) { let main_function = self.main(); - let mut context = InlineContext::new(main_function.entry_block(), main_function.id()); - - let blocks = vecmap(main_function.dfg.basic_blocks_iter(), |(id, _)| id); - - for block in blocks { - let instructions = main_function.dfg[block].instructions(); - - let mut new_instructions = Vec::with_capacity(instructions.len()); - - for (index, instruction) in instructions.iter().copied().enumerate() { - match &main_function.dfg[instruction] { - Instruction::Call { func, arguments } => { - match context.get_function(*func, &main_function.dfg) { - Some(id) => { - main_function.dfg.split_block_at(block, instruction); - context.inline_function(self, id, arguments) - } - None => new_instructions.push(instruction), - } - }, - _ => new_instructions.push(instruction), - } - } - } + let mut context = InlineContext::new(main_function); + let main_id = main_function.id(); + context.inline_function(self, main_id, &[]) } } +/// The context for the function inlining pass. +/// +/// This works using an internal FunctionBuilder to build a new main function from scratch. +/// Doing it this way properly handles importing instructions between functions and lets us +/// reuse the existing API at the cost of essentially cloning each of main's instructions. struct InlineContext { recursion_level: u32, - argument_values: Vec>, - current_block: BasicBlockId, - visited_blocks: HashSet, functions_to_keep: HashSet, + builder: FunctionBuilder, +} + +/// The per-function inlining context contains information that is only valid for one function. +/// For example, each function has its own DataFlowGraph, and thus each function needs a translation +/// layer to translate between BlockId to BlockId for the current function and the function to +/// inline into. The same goes for ValueIds, InstructionIds, and for storing other data like +/// parameter to argument mappings. +struct PerFunctionContext<'function> { + /// The source function is the function we're currently inlining into the function being built. + source_function: &'function Function, + + /// The shared inlining context for all functions. This notably contains the FunctionBuilder used + /// to build the function we're inlining into. + context: &'function mut InlineContext, + + /// Maps ValueIds in the function being inlined to the new ValueIds to use in the function + /// being inlined into. This mapping also contains the mapping from parameter values to + /// argument values. + values: HashMap, + + /// Maps BasicBlockIds in the function being inlined to the new BasicBlockIds to use in the + /// function being inlined into. + blocks: HashMap, + + /// Maps InstructionIds from the function being inlined to the function being inlined into. + instructions: HashMap, } impl InlineContext { /// Create a new context object for the function inlining pass. /// This starts off with an empty mapping of instructions for main's parameters. - fn new(current_block: BasicBlockId, main: FunctionId) -> InlineContext { - let mut visited_blocks = HashSet::new(); - visited_blocks.insert(current_block); + /// + /// main: The main function of the program to start inlining into. Only this function + /// and all functions still reachable from it will be returned when inlining is finished. + fn new(main: &Function) -> InlineContext { + Self { + recursion_level: 0, + builder: FunctionBuilder::new(main.name().to_owned(), main.id()), + functions_to_keep: HashSet::new(), + } + } - let mut functions_to_keep = HashSet::new(); - functions_to_keep.insert(main); + fn inline_function(&mut self, ssa: &Ssa, id: FunctionId, arguments: &[ValueId]) { + self.recursion_level += 1; + + if self.recursion_level > RECURSION_LIMIT { + panic!( + "Attempted to recur more than {RECURSION_LIMIT} times during function inlining." + ); + } + + let source_function = &ssa.functions[&id]; + let mut context = PerFunctionContext::new(self, source_function, arguments); + context.inline_blocks(ssa); + } +} + +impl<'function> PerFunctionContext<'function> { + /// Create a new PerFunctionContext from the source function. + /// The value and block mappings for this context are initially empty except + /// for containing the mapping between parameters in the source_function and + /// the arguments of the destination function. + fn new( + context: &'function mut InlineContext, + source_function: &'function Function, + arguments: &[ValueId], + ) -> Self { + let entry = source_function.entry_block(); + let parameters = source_function.dfg.block_parameters(entry); + assert_eq!(parameters.len(), arguments.len()); Self { - recursion_level: 0, - argument_values: vec![HashMap::new()], - current_block, - visited_blocks, - functions_to_keep, + context, + source_function, + values: parameters.iter().copied().zip(arguments.iter().copied()).collect(), + blocks: HashMap::new(), + instructions: HashMap::new(), } } - fn current_function_arguments(&self) -> &HashMap { - self.argument_values.last() - .expect("Expected there to always be argument values for the current function being inlined") + /// Translates a ValueId from the function being inlined to a ValueId of the function + /// being inlined into. Note that this expects value ids for all Value::Instruction and + /// Value::Param values are already handled as a result of previous inlining of instructions + /// and blocks respectively. If these assertions trigger it means a value is being used before + /// the instruction or block that defines the value is inserted. + fn translate_value(&mut self, id: ValueId) -> ValueId { + if let Some(value) = self.values.get(&id) { + return *value; + } + + let new_value = match &self.source_function.dfg[id] { + Value::Instruction { .. } => { + unreachable!("All Value::Instructions should already be known during inlining after creating the original inlined instruction") + } + Value::Param { .. } => { + unreachable!("All Value::Params should already be known from previous calls to translate_block") + } + Value::NumericConstant { constant, typ } => { + let value = self.source_function.dfg[*constant].value(); + self.context.builder.numeric_constant(value, *typ) + } + Value::Function(function) => self.context.builder.import_function(*function), + Value::Intrinsic(intrinsic) => self.context.builder.import_intrinsic_id(*intrinsic), + }; + + self.values.insert(id, new_value); + new_value } - fn get_function(&self, mut id: ValueId, dfg: &DataFlowGraph) -> Option { - if let Some(new_id) = self.current_function_arguments().get(&id) { - id = *new_id; + /// Translate a block id from the source function to one of the target function. + /// + /// If the block isn't already known, this will insert a new block into the target function + /// with the same parameter types as the source block. + fn translate_block(&mut self, id: BasicBlockId) -> BasicBlockId { + if let Some(block) = self.blocks.get(&id) { + return *block; + } + + // The block is not already present in the function being inlined into so we must create it. + // The block's instructions are not copied over as they will be copied later in inlining. + let new_block = self.context.builder.insert_block(); + let original_parameters = self.source_function.dfg.block_parameters(id); + + for parameter in original_parameters { + let typ = self.source_function.dfg.type_of_value(*parameter); + let new_parameter = self.context.builder.add_block_parameter(new_block, typ); + self.values.insert(*parameter, new_parameter); } - match dfg[id] { + new_block + } + + /// Try to retrieve the function referred to by the given Id. + /// Expects that the given ValueId belongs to the source_function. + /// + /// Returns None if the id is not known to refer to a function. + fn get_function(&mut self, mut id: ValueId) -> Option { + id = self.translate_value(id); + match self.context.builder[id] { Value::Function(id) => Some(id), _ => None, } } - fn inline_function(&mut self, ssa: &Ssa, id: FunctionId, arguments: &[ValueId]) { - let target_function = &ssa.functions[&id]; - let current_block = target_function.entry_block(); - - let parameters = target_function.dfg.block_parameters(current_block); - assert_eq!(parameters.len(), arguments.len()); + /// Inline all reachable blocks within the source_function into the destination function. + fn inline_blocks(&mut self, ssa: &Ssa) { + let mut seen_blocks = HashSet::new(); + let mut block_queue = vec![self.source_function.entry_block()]; - let argument_map = parameters.iter().copied().zip(arguments.iter().copied()).collect(); - self.argument_values.push(argument_map); + while let Some(block_id) = block_queue.pop() { + self.context.builder.switch_to_block(block_id); + seen_blocks.insert(block_id); - let instructions = target_function.dfg[current_block].instructions(); - - let mut new_instructions = Vec::with_capacity(instructions.len()); + self.inline_block(ssa, block_id); + self.handle_terminator_instruction(block_id); + } + } - for id in instructions { - match &target_function.dfg[*id] { - Instruction::Call { func, arguments } => { - match self.get_function(*func, &target_function.dfg) { - Some(id) => self.inline_function(ssa, id, arguments), - None => new_instructions.push(*id), - } + /// Inline each instruction in the given block into the function being inlined into. + /// This may recurse if it finds another function to inline if a call instruction is within this block. + fn inline_block(&mut self, ssa: &Ssa, block_id: BasicBlockId) { + let block = &self.source_function.dfg[block_id]; + for id in block.instructions() { + match &self.source_function.dfg[*id] { + Instruction::Call { func, arguments } => match self.get_function(*func) { + Some(id) => self.context.inline_function(ssa, id, arguments), + None => self.push_instruction(*id), }, - _ => new_instructions.push(*id), + _ => self.push_instruction(*id), } } + } + + /// Push the given instruction from the source_function into the current block of the + /// function being inlined into. + fn push_instruction(&mut self, id: InstructionId) { + let instruction = self.source_function.dfg[id].map_values(|id| self.translate_value(id)); + let results = self.source_function.dfg.instruction_results(id); + + let ctrl_typevars = instruction + .requires_ctrl_typevars() + .then(|| vecmap(results, |result| self.source_function.dfg.type_of_value(*result))); + + let new_results = self.context.builder.insert_instruction(instruction, ctrl_typevars); + + assert_eq!(results.len(), new_results.len()); + for (result, new_result) in results.iter().zip(new_results) { + self.values.insert(*result, *new_result); + } + } - self.argument_values.pop(); + /// Handle the given terminator instruction from the given source function block. + /// This will push any new blocks to the destination function as needed, add them + /// to the block queue, and set the terminator instruction for the current block. + fn handle_terminator_instruction(&mut self, block_id: BasicBlockId) { + match self.source_function.dfg[block_id].terminator() { + Some(TerminatorInstruction::Jmp { destination, arguments }) => { + let destination = self.translate_block(*destination); + let arguments = vecmap(arguments, |arg| self.translate_value(*arg)); + self.context.builder.terminate_with_jmp(destination, arguments); + } + Some(TerminatorInstruction::JmpIf { + condition, + then_destination, + else_destination, + }) => todo!(), + Some(TerminatorInstruction::Return { .. }) => (), + None => unreachable!("Block has no terminator instruction"), + } } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index aa67cbed58..840f2f1c4e 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -9,7 +9,10 @@ use crate::ssa_refactor::ir::{ }; use super::{ - ir::instruction::{InstructionId, Intrinsic}, + ir::{ + basic_block::BasicBlock, + instruction::{InstructionId, Intrinsic}, + }, ssa_gen::Ssa, }; @@ -96,7 +99,7 @@ impl FunctionBuilder { } /// Inserts a new instruction at the end of the current block and returns its results - fn insert_instruction( + pub(crate) fn insert_instruction( &mut self, instruction: Instruction, ctrl_typevars: Option>, @@ -228,8 +231,12 @@ impl FunctionBuilder { /// Retrieve a value reference to the given intrinsic operation. /// Returns None if there is no intrinsic matching the given name. pub(crate) fn import_intrinsic(&mut self, name: &str) -> Option { - Intrinsic::lookup(name) - .map(|intrinsic| self.current_function.dfg.import_intrinsic(intrinsic)) + Intrinsic::lookup(name).map(|intrinsic| self.import_intrinsic_id(intrinsic)) + } + + /// Retrieve a value reference to the given intrinsic operation. + pub(crate) fn import_intrinsic_id(&mut self, intrinsic: Intrinsic) -> ValueId { + self.current_function.dfg.import_intrinsic(intrinsic) } /// Removes the given instruction from the current block or panics otherwise. @@ -253,3 +260,11 @@ impl std::ops::Index for FunctionBuilder { &self.current_function.dfg[id] } } + +impl std::ops::Index for FunctionBuilder { + type Output = BasicBlock; + + fn index(&self, id: BasicBlockId) -> &Self::Output { + &self.current_function.dfg[id] + } +} diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs index 22bff853f0..3c9201100a 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs @@ -8,7 +8,7 @@ use crate::ssa_refactor::ir::function::{Function, FunctionId}; /// /// It is expected that the main function is always the first /// function in the functions vector. -pub struct Ssa { +pub(crate) struct Ssa { pub functions: BTreeMap, } From a103b1e8a999d89ae9600a9b2eeb4f19b1a06e85 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 4 May 2023 10:39:56 -0500 Subject: [PATCH 3/5] Finish function inlining pass --- .../src/ssa_refactor/ir/function.rs | 7 + .../src/ssa_refactor/ir/map.rs | 6 + .../src/ssa_refactor/opt/inlining.rs | 152 +++++++++++------- .../src/ssa_refactor/ssa_builder/mod.rs | 5 + .../src/ssa_refactor/ssa_gen/program.rs | 25 ++- 5 files changed, 134 insertions(+), 61 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs index 97c84942bb..f37448462b 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/function.rs @@ -2,6 +2,7 @@ use super::basic_block::BasicBlockId; use super::dfg::DataFlowGraph; use super::map::Id; use super::types::Type; +use super::value::ValueId; /// A function holds a list of instructions. /// These instructions are further grouped into Basic blocks @@ -54,6 +55,12 @@ impl Function { pub(crate) fn entry_block(&self) -> BasicBlockId { self.entry_block } + + /// Returns the parameters of this function. + /// The parameters will always match that of this function's entry block. + pub(crate) fn parameters(&self) -> &[ValueId] { + self.dfg.block_parameters(self.entry_block) + } } /// FunctionId is a reference for a function diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs index eee1b593ed..43baf4430c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/map.rs @@ -284,6 +284,12 @@ pub(crate) struct AtomicCounter { } impl AtomicCounter { + /// Create a new counter starting after the given Id. + /// Use AtomicCounter::default() to start at zero. + pub(crate) fn starting_after(id: Id) -> Self { + Self { next: AtomicUsize::new(id.index + 1), _marker: Default::default() } + } + /// Return the next fresh id pub(crate) fn next(&self) -> Id { Id::new(self.next.fetch_add(1, Ordering::Relaxed)) diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index 44c7847b16..8a940ba53c 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -1,3 +1,7 @@ +//! This module defines the function inlining pass for the SSA IR. +//! The purpose of this pass is to inline the instructions of each function call +//! within the function caller. If all function calls are known, there will only +//! be a single function remaining when the pass finishes. use std::collections::{HashMap, HashSet}; use iter_extended::vecmap; @@ -31,10 +35,7 @@ impl Ssa { /// pass, we would need to re-run all of inlining anyway to inline it, so we might /// as well save the work for later instead of performing it twice. pub(crate) fn inline_functions(self) -> Ssa { - let main_function = self.main(); - let mut context = InlineContext::new(main_function); - context.inline_main(&self); - context.finish(self) + InlineContext::new(&self).inline_all(self) } } @@ -45,8 +46,12 @@ impl Ssa { /// reuse the existing API at the cost of essentially cloning each of main's instructions. struct InlineContext { recursion_level: u32, - functions_to_keep: HashSet, builder: FunctionBuilder, + + /// True if we failed to inline at least one call. If this is still false when finishing + /// inlining we can remove all other functions from the resulting Ssa struct and keep only + /// the function that was inlined into. + failed_to_inline_a_call: bool, } /// The per-function inlining context contains information that is only valid for one function. @@ -77,26 +82,44 @@ struct PerFunctionContext<'function> { /// The TerminatorInstruction::Return in the source_function will be mapped to a jmp to /// this block in the destination function instead. return_destination: BasicBlockId, + + /// True if we're currently working on the main function. + inlining_main: bool, } impl InlineContext { /// Create a new context object for the function inlining pass. /// This starts off with an empty mapping of instructions for main's parameters. - /// - /// main: The main function of the program to start inlining into. Only this function - /// and all functions still reachable from it will be returned when inlining is finished. - fn new(main: &Function) -> InlineContext { - Self { - recursion_level: 0, - builder: FunctionBuilder::new(main.name().to_owned(), main.id()), - functions_to_keep: HashSet::new(), - } + /// The function being inlined into will always be the main function, although it is + /// actually a copy that is created in case the original main is still needed from a function + /// that could not be inlined calling it. + fn new(ssa: &Ssa) -> InlineContext { + let main_name = ssa.main().name().to_owned(); + let builder = FunctionBuilder::new(main_name, ssa.next_id.next()); + Self { builder, recursion_level: 0, failed_to_inline_a_call: false } } - fn inline_main(&mut self, ssa: &Ssa) { + /// Start inlining the main function and all functions reachable from it. + fn inline_all(mut self, ssa: Ssa) -> Ssa { let main = ssa.main(); - let mut context = PerFunctionContext::new(self, main); - context.inline_blocks(ssa); + let mut context = PerFunctionContext::new(&mut self, main); + context.inlining_main = true; + + // The main block is already inserted so we have to add it to context.blocks and add + // its parameters here. Failing to do so would cause context.translate_block() to add + // a fresh block for the entry block rather than use the existing one. + let entry_block = context.context.builder.current_function.entry_block(); + let original_parameters = context.source_function.parameters(); + + for parameter in original_parameters { + let typ = context.source_function.dfg.type_of_value(*parameter); + let new_parameter = context.context.builder.add_block_parameter(entry_block, typ); + context.values.insert(*parameter, new_parameter); + } + + context.blocks.insert(context.source_function.entry_block(), entry_block); + context.inline_blocks(&ssa); + self.finish(ssa) } /// Inlines a function into the current function and returns the translated return values @@ -113,30 +136,36 @@ impl InlineContext { let source_function = &ssa.functions[&id]; let mut context = PerFunctionContext::new(self, source_function); - let entry = source_function.entry_block(); - let parameters = source_function.dfg.block_parameters(entry); + let parameters = source_function.parameters(); assert_eq!(parameters.len(), arguments.len()); context.values = parameters.iter().copied().zip(arguments.iter().copied()).collect(); + let current_block = context.context.builder.current_block(); + context.blocks.insert(source_function.entry_block(), current_block); + context.inline_blocks(ssa); let return_destination = context.return_destination; self.builder.block_parameters(return_destination) } + /// Finish inlining and return the new Ssa struct with the inlined version of main. + /// If any functions failed to inline, they are not removed from the final Ssa struct. fn finish(self, mut ssa: Ssa) -> Ssa { let mut new_ssa = self.builder.finish(); - - for function_id in self.functions_to_keep { - let function = ssa - .functions - .remove(&function_id) - .unwrap_or_else(|| panic!("Expected to remove function with id {function_id}")); - - let existing = new_ssa.functions.insert(function.id(), function); - assert!(existing.is_none()); + assert_eq!(new_ssa.functions.len(), 1); + + // If we failed to inline any call, any function may still be reachable so we + // don't remove any from the final program. We could be more precise here and + // do a reachability analysis but it should be fine to keep the extra functions + // around longer if they are not called. + if self.failed_to_inline_a_call { + let new_main = new_ssa.functions.pop_first().unwrap().1; + ssa.main_id = new_main.id(); + ssa.functions.insert(new_main.id(), new_main); + ssa + } else { + new_ssa } - - new_ssa } } @@ -148,15 +177,14 @@ impl<'function> PerFunctionContext<'function> { fn new(context: &'function mut InlineContext, source_function: &'function Function) -> Self { // Create the block to return to but don't insert its parameters until we // have the types of the actual return values later. - let return_destination = context.builder.insert_block(); - Self { + return_destination: context.builder.insert_block(), context, source_function, blocks: HashMap::new(), instructions: HashMap::new(), values: HashMap::new(), - return_destination, + inlining_main: false, } } @@ -193,15 +221,22 @@ impl<'function> PerFunctionContext<'function> { /// /// If the block isn't already known, this will insert a new block into the target function /// with the same parameter types as the source block. - fn translate_block(&mut self, id: BasicBlockId) -> BasicBlockId { - if let Some(block) = self.blocks.get(&id) { + fn translate_block( + &mut self, + source_block: BasicBlockId, + block_queue: &mut Vec, + ) -> BasicBlockId { + if let Some(block) = self.blocks.get(&source_block) { return *block; } + // The block is not yet inlined, queue it + block_queue.push(source_block); + // The block is not already present in the function being inlined into so we must create it. // The block's instructions are not copied over as they will be copied later in inlining. let new_block = self.context.builder.insert_block(); - let original_parameters = self.source_function.dfg.block_parameters(id); + let original_parameters = self.source_function.dfg.block_parameters(source_block); for parameter in original_parameters { let typ = self.source_function.dfg.type_of_value(*parameter); @@ -209,6 +244,7 @@ impl<'function> PerFunctionContext<'function> { self.values.insert(*parameter, new_parameter); } + self.blocks.insert(source_block, new_block); new_block } @@ -220,7 +256,11 @@ impl<'function> PerFunctionContext<'function> { id = self.translate_value(id); match self.context.builder[id] { Value::Function(id) => Some(id), - _ => None, + Value::Intrinsic(_) => None, + _ => { + self.context.failed_to_inline_a_call = true; + None + } } } @@ -230,13 +270,15 @@ impl<'function> PerFunctionContext<'function> { let mut block_queue = vec![self.source_function.entry_block()]; while let Some(source_block_id) = block_queue.pop() { - let translated_block_id = self.translate_block(source_block_id); + let translated_block_id = self.translate_block(source_block_id, &mut block_queue); self.context.builder.switch_to_block(translated_block_id); seen_blocks.insert(source_block_id); self.inline_block(ssa, source_block_id); self.handle_terminator_instruction(source_block_id, &mut block_queue); } + + self.context.builder.switch_to_block(self.return_destination); } /// Inline each instruction in the given block into the function being inlined into. @@ -302,16 +344,9 @@ impl<'function> PerFunctionContext<'function> { block_id: BasicBlockId, block_queue: &mut Vec, ) { - let mut translate_and_queue_block = |this: &mut Self, block| { - if this.blocks.get(block).is_none() { - block_queue.push(*block); - } - this.translate_block(*block) - }; - match self.source_function.dfg[block_id].terminator() { Some(TerminatorInstruction::Jmp { destination, arguments }) => { - let destination = translate_and_queue_block(self, destination); + let destination = self.translate_block(*destination, block_queue); let arguments = vecmap(arguments, |arg| self.translate_value(*arg)); self.context.builder.terminate_with_jmp(destination, arguments); } @@ -321,19 +356,24 @@ impl<'function> PerFunctionContext<'function> { else_destination, }) => { let condition = self.translate_value(*condition); - let then_block = translate_and_queue_block(self, then_destination); - let else_block = translate_and_queue_block(self, else_destination); + let then_block = self.translate_block(*then_destination, block_queue); + let else_block = self.translate_block(*else_destination, block_queue); self.context.builder.terminate_with_jmpif(condition, then_block, else_block); } Some(TerminatorInstruction::Return { return_values }) => { - let return_values = vecmap(return_values, |value| { - // Add the block parameters for the return block here since we don't do - // it when inserting the block in PerFunctionContext::new - let typ = self.source_function.dfg.type_of_value(*value); - self.context.builder.add_block_parameter(self.return_destination, typ); - self.translate_value(*value) - }); - self.context.builder.terminate_with_jmp(self.return_destination, return_values); + let return_values = vecmap(return_values, |value| self.translate_value(*value)); + + if self.inlining_main { + self.context.builder.terminate_with_return(return_values); + } else { + for value in &return_values { + // Add the block parameters for the return block here since we don't do + // it when inserting the block in PerFunctionContext::new + let typ = self.context.builder.current_function.dfg.type_of_value(*value); + self.context.builder.add_block_parameter(self.return_destination, typ); + } + self.context.builder.terminate_with_jmp(self.return_destination, return_values); + } } None => unreachable!("Block has no terminator instruction"), } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index ffb1f7210e..f621503e59 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -121,6 +121,11 @@ impl FunctionBuilder { self.current_block = block; } + /// Returns the block currently being inserted into + pub(crate) fn current_block(&mut self) -> BasicBlockId { + self.current_block + } + /// Insert an allocate instruction at the end of the current block, allocating the /// given amount of field elements. Returns the result of the allocate instruction, /// which is always a Reference to the allocated data. diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs index 3304513b35..7f4b9a8dd2 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_gen/program.rs @@ -2,27 +2,42 @@ use std::{collections::BTreeMap, fmt::Display}; use iter_extended::btree_map; -use crate::ssa_refactor::ir::function::{Function, FunctionId}; +use crate::ssa_refactor::ir::{ + function::{Function, FunctionId}, + map::AtomicCounter, +}; /// Contains the entire SSA representation of the program. pub(crate) struct Ssa { pub(crate) functions: BTreeMap, + pub(crate) main_id: FunctionId, + pub(crate) next_id: AtomicCounter, } impl Ssa { - /// Create a new Ssa object from the given SSA functions + /// Create a new Ssa object from the given SSA functions. + /// The first function in this vector is expected to be the main function. pub(crate) fn new(functions: Vec) -> Self { - Self { functions: btree_map(functions, |f| (f.id(), f)) } + let main_id = functions.first().expect("Expected at least 1 SSA function").id(); + let mut max_id = main_id; + + let functions = btree_map(functions, |f| { + max_id = std::cmp::max(max_id, f.id()); + (f.id(), f) + }); + + Self { functions, main_id, next_id: AtomicCounter::starting_after(max_id) } } + /// Returns the entry-point function of the program pub(crate) fn main(&self) -> &Function { - self.functions.first_key_value().expect("Expected there to be at least 1 SSA function").1 + &self.functions[&self.main_id] } } impl Display for Ssa { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for (_, function) in &self.functions { + for function in self.functions.values() { writeln!(f, "{function}")?; } Ok(()) From 70608fb4b12d74a617493b5b6ed34f1060edfc8f Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 4 May 2023 13:02:39 -0500 Subject: [PATCH 4/5] Add basic test --- .../src/ssa_refactor/ir/dom.rs | 4 +- .../src/ssa_refactor/opt/inlining.rs | 39 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dom.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dom.rs index 588b25cf91..dba656838b 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dom.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dom.rs @@ -278,7 +278,7 @@ mod tests { builder.switch_to_block(block3_id); builder.terminate_with_return(vec![]); - let mut ssa = builder.finish(); + let ssa = builder.finish(); let func = ssa.main(); let block0_id = func.entry_block(); @@ -382,7 +382,7 @@ mod tests { builder.switch_to_block(block2_id); builder.terminate_with_jmp(block1_id, vec![]); - let mut ssa = builder.finish(); + let ssa = builder.finish(); let func = ssa.main(); let block0_id = func.entry_block(); diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index 8a940ba53c..6e7c984874 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -379,3 +379,42 @@ impl<'function> PerFunctionContext<'function> { } } } + +#[cfg(test)] +mod test { + use crate::ssa_refactor::{ + ir::{map::Id, types::Type}, + ssa_builder::FunctionBuilder, + }; + + #[test] + fn basic_inlining() { + // fn foo { + // b0(): + // v0 = call bar() + // return v0 + // } + // fn bar { + // b0(): + // return 72 + // } + let foo_id = Id::test_new(0); + let mut builder = FunctionBuilder::new("foo".into(), foo_id); + + let bar_id = Id::test_new(1); + let bar = builder.import_function(bar_id); + let results = builder.insert_call(bar, Vec::new(), vec![Type::field()]).to_vec(); + builder.terminate_with_return(results); + + builder.new_function("bar".into(), bar_id); + let expected_return = 72u128; + let seventy_two = builder.field_constant(expected_return); + builder.terminate_with_return(vec![seventy_two]); + + let ssa = builder.finish(); + assert_eq!(ssa.functions.len(), 2); + + let inlined = ssa.inline_functions(); + assert_eq!(inlined.functions.len(), 1); + } +} From 4ec74d6cce3077cc2b12c68b00603ca80a5a2da6 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Fri, 5 May 2023 13:49:00 -0500 Subject: [PATCH 5/5] Address PR comments --- .../src/ssa_refactor/ir/basic_block.rs | 2 ++ .../src/ssa_refactor/ir/dfg.rs | 31 ------------------- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs index c110b722d6..30526bc296 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs @@ -96,6 +96,8 @@ impl BasicBlock { /// Removes the given instruction from this block if present or panics otherwise. pub(crate) fn remove_instruction(&mut self, instruction: InstructionId) { + // Iterate in reverse here as an optimization since remove_instruction is most + // often called to remove instructions at the end of a block. let index = self.instructions.iter().rev().position(|id| *id == instruction).unwrap_or_else(|| { panic!("remove_instruction: No such instruction {instruction:?} in block") diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 5ad96e1bc1..3ab345f06b 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -263,37 +263,6 @@ impl DataFlowGraph { ) { self.blocks[block].set_terminator(terminator); } - - /// Splits the given block in two at the given instruction, returning the Id of the new block. - /// This will remove the given instruction and place every instruction after it into a new block - /// with the same terminator as the old block. The old block is modified to stop - /// before the instruction to remove and to unconditionally branch to the new block. - /// This function is useful during function inlining to remove the call instruction - /// while opening a spot at the end of the current block to insert instructions into. - /// - /// Example (before): - /// block1: a; b; c; d; e; jmp block5 - /// - /// After self.split_block_at(block1, c): - /// block1: a; b; jmp block2 - /// block2: d; e; jmp block5 - pub(crate) fn split_block_at( - &mut self, - block: BasicBlockId, - instruction_to_remove: InstructionId, - ) -> BasicBlockId { - let split_block = &mut self.blocks[block]; - - let mut instructions = split_block.instructions().iter(); - let index = instructions.position(|id| *id == instruction_to_remove).unwrap_or_else(|| { - panic!("No instruction found with id {instruction_to_remove:?} in block {block:?}") - }); - - let instructions = split_block.instructions_mut().drain(index..).collect(); - split_block.remove_instruction(instruction_to_remove); - - self.blocks.insert(BasicBlock::new(instructions)) - } } impl std::ops::Index for DataFlowGraph {