From eda50c7b4322a89e53ecf05e820d7b7fd991e48c Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 20 Dec 2024 10:25:43 +0000 Subject: [PATCH 01/11] Use is_brillig and is_acir --- compiler/noirc_evaluator/src/ssa/ir/value.rs | 8 ++++---- .../noirc_evaluator/src/ssa/opt/runtime_separation.rs | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/value.rs b/compiler/noirc_evaluator/src/ssa/ir/value.rs index ec7a8e2524..39c63e3efc 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/value.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/value.rs @@ -20,10 +20,10 @@ pub(crate) type ValueId = Id; pub(crate) enum Value { /// This value was created due to an instruction /// - /// instruction -- This is the instruction which defined it - /// typ -- This is the `Type` of the instruction - /// position -- Returns the position in the results - /// vector that this `Value` is located. + /// * `instruction`: This is the instruction which defined it + /// * `typ`: This is the `Type` of the instruction + /// * `position`: Returns the position in the results vector that this `Value` is located. + /// /// Example, if you add two numbers together, then the resulting /// value would have position `0`, the typ would be the type /// of the operands, and the instruction would map to an add instruction. diff --git a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs index b671d5011a..495b241026 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs @@ -64,13 +64,12 @@ impl RuntimeSeparatorContext { processed_functions: &mut HashSet<(/* within_brillig */ bool, FunctionId)>, ) { // Processed functions needs the within brillig flag, since it is possible to call the same function from both brillig and acir - if processed_functions.contains(&(within_brillig, current_func_id)) { + if !processed_functions.insert((within_brillig, current_func_id)) { return; } - processed_functions.insert((within_brillig, current_func_id)); let func = &ssa.functions[¤t_func_id]; - if matches!(func.runtime(), RuntimeType::Brillig(_)) { + if func.runtime().is_brillig() { within_brillig = true; } @@ -79,7 +78,7 @@ impl RuntimeSeparatorContext { if within_brillig { for called_func_id in called_functions.iter() { let called_func = &ssa.functions[called_func_id]; - if matches!(called_func.runtime(), RuntimeType::Acir(_)) { + if called_func.runtime().is_acir() { self.acir_functions_called_from_brillig.insert(*called_func_id); } } @@ -110,7 +109,7 @@ impl RuntimeSeparatorContext { fn replace_calls_to_mapped_functions(&self, ssa: &mut Ssa) { for (_function_id, func) in ssa.functions.iter_mut() { - if matches!(func.runtime(), RuntimeType::Brillig(_)) { + if func.runtime().is_brillig() { for called_func_value_id in called_functions_values(func).iter() { let Value::Function(called_func_id) = &func.dfg[*called_func_value_id] else { unreachable!("Value should be a function") From d3179622c90f218545b2f954e050d505365d8482 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 20 Dec 2024 12:13:15 +0000 Subject: [PATCH 02/11] Remove Rc instructions after runtime separation --- compiler/noirc_evaluator/src/acir/mod.rs | 3 ++- .../noirc_evaluator/src/ssa/ir/function.rs | 23 +++++++++++++++++-- .../noirc_evaluator/src/ssa/ir/instruction.rs | 6 +++++ .../src/ssa/opt/runtime_separation.rs | 9 ++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 95e0dd1213..a79804e2c2 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -769,7 +769,8 @@ impl<'a> Context<'a> { unreachable!("Expected all load instructions to be removed before acir_gen") } Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. } => { - // Do nothing. Only Brillig needs to worry about reference counted arrays + // Only Brillig needs to worry about reference counted arrays + unreachable!("Expected all Rc instructions to be removed before acir_gen") } Instruction::RangeCheck { value, max_bit_size, assert_message } => { let acir_var = self.convert_numeric_value(*value, dfg)?; diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 6413107c04..dce7c46c1d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashSet}; use iter_extended::vecmap; use noirc_frontend::monomorphization::ast::InlineType; @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use super::basic_block::BasicBlockId; use super::dfg::DataFlowGraph; -use super::instruction::TerminatorInstruction; +use super::instruction::{Instruction, TerminatorInstruction}; use super::map::Id; use super::types::Type; use super::value::ValueId; @@ -195,6 +195,25 @@ impl Function { unreachable!("SSA Function {} has no reachable return instruction!", self.id()) } + + /// Remove instructions from all the reachable blocks in a function based on a predicate, + /// keeping the ones where the predicate returns `true`. + pub(crate) fn retain_instructions(&mut self, pred: impl Fn(&Instruction) -> bool) { + for block_id in self.reachable_blocks() { + let mut to_remove = HashSet::new(); + let block = &self.dfg[block_id]; + for instruction_id in block.instructions() { + let instruction = &self.dfg[*instruction_id]; + if !pred(instruction) { + to_remove.insert(*instruction_id); + } + } + if !to_remove.is_empty() { + let block = &mut self.dfg[block_id]; + block.instructions_mut().retain(|id| !to_remove.contains(id)); + } + } + } } impl Clone for Function { diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 5d9bfc89f6..f1aef99217 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -1037,6 +1037,12 @@ impl Instruction { Instruction::MakeArray { .. } => None, } } + + /// Some instructions are only to be used in Brillig and should be eliminated + /// after runtime separation, never to be be reintroduced in an ACIR runtime. + pub(crate) fn is_brillig_only(&self) -> bool { + matches!(self, Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. }) + } } /// Given a chain of operations like: diff --git a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs index 495b241026..c01421f1b0 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs @@ -21,6 +21,7 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn separate_runtime(mut self) -> Self { RuntimeSeparatorContext::separate_runtime(&mut self); + remove_brillig_only_from_acir(&mut self); self } } @@ -179,6 +180,14 @@ fn prune_unreachable_functions(ssa: &mut Ssa) { ssa.functions.retain(|id, _value| reachable_functions.contains(id)); } +fn remove_brillig_only_from_acir(ssa: &mut Ssa) { + for (_, func) in ssa.functions.iter_mut() { + if func.runtime().is_acir() { + func.retain_instructions(|i| !i.is_brillig_only()); + } + } +} + #[cfg(test)] mod test { use std::collections::BTreeSet; From 8cff5cc943870725c0e405d4ea9278d74c1121e1 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 20 Dec 2024 14:16:33 +0000 Subject: [PATCH 03/11] Move the runtime into the DFG and check during insertion --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 74 +++++++++++++++++-- .../noirc_evaluator/src/ssa/ir/function.rs | 26 +++++-- .../src/ssa/opt/runtime_separation.rs | 14 ++-- 3 files changed, 91 insertions(+), 23 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 9ccf7f1151..3d6b184e3b 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -5,7 +5,7 @@ use crate::ssa::{function_builder::data_bus::DataBus, ir::instruction::SimplifyR use super::{ basic_block::{BasicBlock, BasicBlockId}, call_stack::{CallStack, CallStackHelper, CallStackId}, - function::FunctionId, + function::{FunctionId, RuntimeType}, instruction::{ Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction, }, @@ -27,8 +27,23 @@ use serde_with::DisplayFromStr; /// owning most data in a function and handing out Ids to this data that can be /// shared without worrying about ownership. #[serde_as] -#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct DataFlowGraph { + /// Runtime of the [Function] that owns this [DataFlowGraph]. + /// This might change during the `runtime_separation` pass where + /// ACIR functions are cloned as Brillig functions. + runtime: RuntimeType, + + /// Indicate whether we are past the runtime separation step. + /// Before this step the DFG accepts any instruction, because + /// an ACIR function might be cloned as a Brillig function. + /// After the separation, we can instantly remove instructions + /// that would just be ignored by the runtime. + /// + /// TODO(#6841): This would not be necessary if the SSA was + /// already generated for a specific runtime. + is_runtime_separated: bool, + /// All of the instructions in a function instructions: DenseMap, @@ -101,6 +116,42 @@ pub(crate) struct DataFlowGraph { } impl DataFlowGraph { + pub(crate) fn new(runtime: RuntimeType) -> Self { + Self { + runtime, + is_runtime_separated: false, + instructions: Default::default(), + results: Default::default(), + values: Default::default(), + constants: Default::default(), + functions: Default::default(), + intrinsics: Default::default(), + foreign_functions: Default::default(), + blocks: Default::default(), + replaced_value_ids: Default::default(), + locations: Default::default(), + call_stack_data: Default::default(), + data_bus: Default::default(), + } + } + + /// Runtime type of the function. + pub(crate) fn runtime(&self) -> RuntimeType { + self.runtime + } + + /// Set runtime type of the function. + pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { + self.runtime = runtime; + } + + /// Mark the runtime as separated. After this we can drop + /// instructions that would be ignored by the runtime, + /// instead of inserting them and carrying them to the end. + pub(crate) fn set_runtime_separated(&mut self) { + self.is_runtime_separated = true; + } + /// Creates a new basic block with no parameters. /// After being created, the block is unreachable in the current function /// until another block is made to jump to it. @@ -165,6 +216,11 @@ impl DataFlowGraph { id } + /// Check if the function runtime would simply ignore this instruction. + pub(crate) fn is_handled_by_runtime(&self, instruction: &Instruction) -> bool { + !(self.runtime().is_acir() && instruction.is_brillig_only()) + } + fn insert_instruction_without_simplification( &mut self, instruction_data: Instruction, @@ -178,7 +234,7 @@ impl DataFlowGraph { id } - pub(crate) fn insert_instruction_and_results_without_simplification( + fn insert_instruction_and_results_without_simplification( &mut self, instruction_data: Instruction, block: BasicBlockId, @@ -195,7 +251,8 @@ impl DataFlowGraph { InsertInstructionResult::Results(id, self.instruction_results(id)) } - /// Inserts a new instruction at the end of the given block and returns its results + /// Simplifies a new instruction and inserts it at the end of the given block and returns its results. + /// If the instruction is not handled by the current runtime, `InstructionRemoved` is returned. pub(crate) fn insert_instruction_and_results( &mut self, instruction: Instruction, @@ -203,6 +260,9 @@ impl DataFlowGraph { ctrl_typevars: Option>, call_stack: CallStackId, ) -> InsertInstructionResult { + if self.is_runtime_separated && !self.is_handled_by_runtime(&instruction) { + return InsertInstructionResult::InstructionRemoved; + } match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) { SimplifyResult::SimplifiedTo(simplification) => { InsertInstructionResult::SimplifiedTo(simplification) @@ -694,12 +754,14 @@ impl<'dfg> std::ops::Index for InsertInstructionResult<'dfg> { #[cfg(test)] mod tests { + use noirc_frontend::monomorphization::ast::InlineType; + use super::DataFlowGraph; - use crate::ssa::ir::{instruction::Instruction, types::Type}; + use crate::ssa::ir::{dfg::RuntimeType, instruction::Instruction, types::Type}; #[test] fn make_instruction() { - let mut dfg = DataFlowGraph::default(); + let mut dfg = DataFlowGraph::new(RuntimeType::Acir(InlineType::Inline)); let ins = Instruction::Allocate; let ins_id = dfg.make_instruction(ins, Some(vec![Type::field()])); diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index dce7c46c1d..d70338cdcd 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -72,8 +72,6 @@ pub(crate) struct Function { id: FunctionId, - runtime: RuntimeType, - /// The DataFlowGraph holds the majority of data pertaining to the function /// including its blocks, instructions, and values. pub(crate) dfg: DataFlowGraph, @@ -84,22 +82,22 @@ 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(); + let mut dfg = DataFlowGraph::new(RuntimeType::Acir(InlineType::default())); let entry_block = dfg.make_block(); - Self { name, id, entry_block, dfg, runtime: RuntimeType::Acir(InlineType::default()) } + Self { name, id, entry_block, dfg } } /// Creates a new function as a clone of the one passed in with the passed in id. pub(crate) fn clone_with_id(id: FunctionId, another: &Function) -> Self { let dfg = another.dfg.clone(); let entry_block = another.entry_block; - Self { name: another.name.clone(), id, entry_block, dfg, runtime: another.runtime } + Self { name: another.name.clone(), id, entry_block, dfg } } /// Takes the signature (function name & runtime) from a function but does not copy the body. pub(crate) fn clone_signature(id: FunctionId, another: &Function) -> Self { let mut new_function = Function::new(another.name.clone(), id); - new_function.runtime = another.runtime; + new_function.set_runtime(another.runtime()); new_function } @@ -116,12 +114,12 @@ impl Function { /// Runtime type of the function. pub(crate) fn runtime(&self) -> RuntimeType { - self.runtime + self.dfg.runtime() } /// Set runtime type of the function. pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { - self.runtime = runtime; + self.dfg.set_runtime(runtime); } pub(crate) fn is_no_predicates(&self) -> bool { @@ -214,6 +212,18 @@ impl Function { } } } + + /// Remove all instructions that aren't handled by the runtime, which is now + /// considered final. Seal the [DataFlowGraph] so it instantly simplifies + /// away instructions that the runtime would have to ignore. + /// + /// TODO(#6841): Remove once the SSA generated is for a specific runtime already. + pub(crate) fn separate_runtime(&mut self) { + if self.runtime().is_acir() { + self.retain_instructions(|i| !i.is_brillig_only()); + } + self.dfg.set_runtime_separated(); + } } impl Clone for Function { diff --git a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs index c01421f1b0..e3d271a15b 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs @@ -21,7 +21,6 @@ impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn separate_runtime(mut self) -> Self { RuntimeSeparatorContext::separate_runtime(&mut self); - remove_brillig_only_from_acir(&mut self); self } } @@ -55,6 +54,11 @@ impl RuntimeSeparatorContext { // Some functions might be unreachable now (for example an acir function only called from brillig) prune_unreachable_functions(ssa); + + // Finally we can mark each function as having their runtimes be separated. + for func in ssa.functions.values_mut() { + func.separate_runtime(); + } } fn collect_acir_functions_called_from_brillig( @@ -180,14 +184,6 @@ fn prune_unreachable_functions(ssa: &mut Ssa) { ssa.functions.retain(|id, _value| reachable_functions.contains(id)); } -fn remove_brillig_only_from_acir(ssa: &mut Ssa) { - for (_, func) in ssa.functions.iter_mut() { - if func.runtime().is_acir() { - func.retain_instructions(|i| !i.is_brillig_only()); - } - } -} - #[cfg(test)] mod test { use std::collections::BTreeSet; From e41bd9aac133c19e6dcccbc2941da6e016234d7b Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Fri, 20 Dec 2024 15:55:45 +0000 Subject: [PATCH 04/11] Add separated to set_runtime --- .../src/brillig/brillig_gen/brillig_slice_ops.rs | 2 +- .../src/brillig/brillig_gen/variable_liveness.rs | 6 +++--- .../noirc_evaluator/src/ssa/function_builder/mod.rs | 11 ++++++----- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 8 +++++++- compiler/noirc_evaluator/src/ssa/ir/function.rs | 11 ++++++++--- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 6 +++--- compiler/noirc_evaluator/src/ssa/opt/rc.rs | 2 +- .../noirc_evaluator/src/ssa/opt/runtime_separation.rs | 4 ++-- compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs | 3 ++- compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs | 2 +- 10 files changed, 34 insertions(+), 21 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs index 26c7151bf0..320cac602c 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs @@ -180,7 +180,7 @@ mod tests { fn create_test_environment() -> (Ssa, FunctionContext, BrilligContext) { let mut builder = FunctionBuilder::new("main".to_string(), Id::test_new(0)); - builder.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); let ssa = builder.finish(); let mut brillig_context = create_context(ssa.main_id); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs index d6851a9ecf..6884c9a67c 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs @@ -361,7 +361,7 @@ mod test { let main_id = Id::test_new(1); let mut builder = FunctionBuilder::new("main".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); let b1 = builder.insert_block(); let b2 = builder.insert_block(); @@ -471,7 +471,7 @@ mod test { let main_id = Id::test_new(1); let mut builder = FunctionBuilder::new("main".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); let b1 = builder.insert_block(); let b2 = builder.insert_block(); @@ -610,7 +610,7 @@ mod test { let main_id = Id::test_new(1); let mut builder = FunctionBuilder::new("main".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); let v0 = builder.add_parameter(Type::bool()); diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 855034cedd..95d8fd29d0 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -63,9 +63,9 @@ impl FunctionBuilder { /// the FunctionBuilder. A function's default runtime type is `RuntimeType::Acir(InlineType::Inline)`. /// This should only be used immediately following construction of a FunctionBuilder /// and will panic if there are any already finished functions. - pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { + pub(crate) fn set_runtime(&mut self, runtime: RuntimeType, separated: bool) { assert_eq!(self.finished_functions.len(), 0, "Attempted to set runtime on a FunctionBuilder with finished functions. A FunctionBuilder's runtime should only be set on its initial function"); - self.current_function.set_runtime(runtime); + self.current_function.set_runtime(runtime, separated); } /// Finish the current function and create a new function. @@ -78,10 +78,11 @@ impl FunctionBuilder { name: String, function_id: FunctionId, runtime_type: RuntimeType, + separated: bool, ) { 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); + new_function.set_runtime(runtime_type, separated); 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 @@ -97,7 +98,7 @@ impl FunctionBuilder { function_id: FunctionId, inline_type: InlineType, ) { - self.new_function_with_type(name, function_id, RuntimeType::Acir(inline_type)); + self.new_function_with_type(name, function_id, RuntimeType::Acir(inline_type), false); } /// Finish the current function and create a new unconstrained function. @@ -107,7 +108,7 @@ impl FunctionBuilder { function_id: FunctionId, inline_type: InlineType, ) { - self.new_function_with_type(name, function_id, RuntimeType::Brillig(inline_type)); + self.new_function_with_type(name, function_id, RuntimeType::Brillig(inline_type), false); } /// Consume the FunctionBuilder returning all the functions it has generated. diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 3d6b184e3b..58c5d0e6e5 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -140,9 +140,15 @@ impl DataFlowGraph { self.runtime } + /// Indicate whether the runtimes have been separated yet. + pub(crate) fn is_runtime_separated(&self) -> bool { + self.is_runtime_separated + } + /// Set runtime type of the function. - pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { + pub(crate) fn set_runtime(&mut self, runtime: RuntimeType, separated: bool) { self.runtime = runtime; + self.is_runtime_separated = separated; } /// Mark the runtime as separated. After this we can drop diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index d70338cdcd..07353695ec 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -97,7 +97,7 @@ impl Function { /// Takes the signature (function name & runtime) from a function but does not copy the body. pub(crate) fn clone_signature(id: FunctionId, another: &Function) -> Self { let mut new_function = Function::new(another.name.clone(), id); - new_function.set_runtime(another.runtime()); + new_function.set_runtime(another.runtime(), another.is_runtime_separated()); new_function } @@ -117,9 +117,14 @@ impl Function { self.dfg.runtime() } + /// Indicate whether the runtimes have been separated yet. + pub(crate) fn is_runtime_separated(&self) -> bool { + self.dfg.is_runtime_separated() + } + /// Set runtime type of the function. - pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { - self.dfg.set_runtime(runtime); + pub(crate) fn set_runtime(&mut self, runtime: RuntimeType, separated: bool) { + self.dfg.set_runtime(runtime, separated); } pub(crate) fn is_no_predicates(&self) -> bool { diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 11201fc8f8..b0b090dcaa 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -361,7 +361,7 @@ impl InlineContext { ) -> InlineContext { let source = &ssa.functions[&entry_point]; let mut builder = FunctionBuilder::new(source.name().to_owned(), entry_point); - builder.set_runtime(source.runtime()); + builder.set_runtime(source.runtime(), source.is_runtime_separated()); Self { builder, recursion_level: 0, @@ -1163,7 +1163,7 @@ mod test { // } let foo_id = Id::test_new(0); let mut builder = FunctionBuilder::new("foo".into(), foo_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.set_runtime(RuntimeType::Brillig(InlineType::default()), true); let bar_id = Id::test_new(1); let bar = builder.import_function(bar_id); @@ -1205,7 +1205,7 @@ mod test { // } let foo_id = Id::test_new(0); let mut builder = FunctionBuilder::new("foo".into(), foo_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.set_runtime(RuntimeType::Brillig(InlineType::default()), true); let bar_id = Id::test_new(1); let bar = builder.import_function(bar_id); diff --git a/compiler/noirc_evaluator/src/ssa/opt/rc.rs b/compiler/noirc_evaluator/src/ssa/opt/rc.rs index 64f6e2ddfe..18bf36f3f4 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/rc.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/rc.rs @@ -206,7 +206,7 @@ mod test { // } let main_id = Id::test_new(0); let mut builder = FunctionBuilder::new("foo".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); let inner_array_type = Type::Array(Arc::new(vec![Type::field()]), 2); let v0 = builder.add_parameter(inner_array_type.clone()); diff --git a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs index e3d271a15b..15c7e63e23 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/runtime_separation.rs @@ -107,7 +107,7 @@ impl RuntimeSeparatorContext { let cloned_id = ssa.clone_fn(*acir_func_id); let new_func = ssa.functions.get_mut(&cloned_id).expect("Cloned function should exist in SSA"); - new_func.set_runtime(RuntimeType::Brillig(inline_type)); + new_func.set_runtime(RuntimeType::Brillig(inline_type), true); self.mapped_functions.insert(*acir_func_id, cloned_id); } } @@ -214,7 +214,7 @@ mod test { // } let foo_id = Id::test_new(0); let mut builder = FunctionBuilder::new("foo".into(), foo_id); - builder.current_function.set_runtime(RuntimeType::Brillig(InlineType::default())); + builder.current_function.set_runtime(RuntimeType::Brillig(InlineType::default()), false); let bar_id = Id::test_new(1); let bar = builder.import_function(bar_id); diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 7c7e977c6c..8984962ba3 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -59,7 +59,8 @@ impl Translator { let main_function = parsed_ssa.functions.remove(0); let main_id = FunctionId::test_new(0); let mut builder = FunctionBuilder::new(main_function.external_name.clone(), main_id); - builder.set_runtime(main_function.runtime_type); + let separated = false; // Allow the builder to do anything it wants. + builder.set_runtime(main_function.runtime_type, separated); // Map function names to their IDs so calls can be resolved let mut function_id_counter = 1; diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 7807658dab..b2ef80c378 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -107,7 +107,7 @@ impl<'a> FunctionContext<'a> { .1; let mut builder = FunctionBuilder::new(function_name, function_id); - builder.set_runtime(runtime); + builder.set_runtime(runtime, false); let definitions = HashMap::default(); let mut this = Self { definitions, builder, shared_context, loops: Vec::new() }; this.add_parameters_to_scope(parameters); From a5f6157543b15690dec4155f91018589f50abf63 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 6 Jan 2025 15:19:06 +0000 Subject: [PATCH 05/11] Make the non-simplifying method public --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index eb10bf2559..2be3eba488 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -239,13 +239,17 @@ impl DataFlowGraph { id } - fn insert_instruction_and_results_without_simplification( + pub(crate) fn insert_instruction_and_results_without_simplification( &mut self, instruction_data: Instruction, block: BasicBlockId, ctrl_typevars: Option>, call_stack: CallStackId, ) -> InsertInstructionResult { + if self.is_runtime_separated && !self.is_handled_by_runtime(&instruction_data) { + return InsertInstructionResult::InstructionRemoved; + } + let id = self.insert_instruction_without_simplification( instruction_data, block, From b9598bbb596d49fea743541eddb5566287d5d0cf Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 6 Jan 2025 19:28:50 +0000 Subject: [PATCH 06/11] . --- .../src/ssa/function_builder/mod.rs | 11 ++--- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 30 ++---------- .../noirc_evaluator/src/ssa/ir/function.rs | 46 ++----------------- .../noirc_evaluator/src/ssa/opt/inlining.rs | 2 +- .../src/ssa/ssa_gen/context.rs | 2 +- 5 files changed, 15 insertions(+), 76 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index 76c5d13ce1..d08d533923 100644 --- a/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -68,9 +68,9 @@ impl FunctionBuilder { /// the FunctionBuilder. A function's default runtime type is `RuntimeType::Acir(InlineType::Inline)`. /// This should only be used immediately following construction of a FunctionBuilder /// and will panic if there are any already finished functions. - pub(crate) fn set_runtime(&mut self, runtime: RuntimeType, separated: bool) { + pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { assert_eq!(self.finished_functions.len(), 0, "Attempted to set runtime on a FunctionBuilder with finished functions. A FunctionBuilder's runtime should only be set on its initial function"); - self.current_function.set_runtime(runtime, separated); + self.current_function.set_runtime(runtime); } /// Finish the current function and create a new function. @@ -83,11 +83,10 @@ impl FunctionBuilder { name: String, function_id: FunctionId, runtime_type: RuntimeType, - separated: bool, ) { 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, separated); + 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 @@ -103,7 +102,7 @@ impl FunctionBuilder { function_id: FunctionId, inline_type: InlineType, ) { - self.new_function_with_type(name, function_id, RuntimeType::Acir(inline_type), false); + self.new_function_with_type(name, function_id, RuntimeType::Acir(inline_type)); } /// Finish the current function and create a new unconstrained function. @@ -113,7 +112,7 @@ impl FunctionBuilder { function_id: FunctionId, inline_type: InlineType, ) { - self.new_function_with_type(name, function_id, RuntimeType::Brillig(inline_type), false); + self.new_function_with_type(name, function_id, RuntimeType::Brillig(inline_type)); } /// Consume the FunctionBuilder returning all the functions it has generated. diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 2be3eba488..83f81a038a 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -33,16 +33,6 @@ pub(crate) struct DataFlowGraph { /// ACIR functions are cloned as Brillig functions. runtime: RuntimeType, - /// Indicate whether we are past the runtime separation step. - /// Before this step the DFG accepts any instruction, because - /// an ACIR function might be cloned as a Brillig function. - /// After the separation, we can instantly remove instructions - /// that would just be ignored by the runtime. - /// - /// TODO(#6841): This would not be necessary if the SSA was - /// already generated for a specific runtime. - is_runtime_separated: bool, - /// All of the instructions in a function instructions: DenseMap, @@ -118,7 +108,6 @@ impl DataFlowGraph { pub(crate) fn new(runtime: RuntimeType) -> Self { Self { runtime, - is_runtime_separated: false, instructions: Default::default(), results: Default::default(), values: Default::default(), @@ -139,22 +128,9 @@ impl DataFlowGraph { self.runtime } - /// Indicate whether the runtimes have been separated yet. - pub(crate) fn is_runtime_separated(&self) -> bool { - self.is_runtime_separated - } - /// Set runtime type of the function. - pub(crate) fn set_runtime(&mut self, runtime: RuntimeType, separated: bool) { + pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { self.runtime = runtime; - self.is_runtime_separated = separated; - } - - /// Mark the runtime as separated. After this we can drop - /// instructions that would be ignored by the runtime, - /// instead of inserting them and carrying them to the end. - pub(crate) fn set_runtime_separated(&mut self) { - self.is_runtime_separated = true; } /// Creates a new basic block with no parameters. @@ -246,7 +222,7 @@ impl DataFlowGraph { ctrl_typevars: Option>, call_stack: CallStackId, ) -> InsertInstructionResult { - if self.is_runtime_separated && !self.is_handled_by_runtime(&instruction_data) { + if !self.is_handled_by_runtime(&instruction_data) { return InsertInstructionResult::InstructionRemoved; } @@ -269,7 +245,7 @@ impl DataFlowGraph { ctrl_typevars: Option>, call_stack: CallStackId, ) -> InsertInstructionResult { - if self.is_runtime_separated && !self.is_handled_by_runtime(&instruction) { + if !self.is_handled_by_runtime(&instruction) { return InsertInstructionResult::InstructionRemoved; } match instruction.simplify(self, block, ctrl_typevars.clone(), call_stack) { diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 07353695ec..25c3573fbd 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeSet, HashSet}; +use std::collections::BTreeSet; use iter_extended::vecmap; use noirc_frontend::monomorphization::ast::InlineType; @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use super::basic_block::BasicBlockId; use super::dfg::DataFlowGraph; -use super::instruction::{Instruction, TerminatorInstruction}; +use super::instruction::TerminatorInstruction; use super::map::Id; use super::types::Type; use super::value::ValueId; @@ -97,7 +97,7 @@ impl Function { /// Takes the signature (function name & runtime) from a function but does not copy the body. pub(crate) fn clone_signature(id: FunctionId, another: &Function) -> Self { let mut new_function = Function::new(another.name.clone(), id); - new_function.set_runtime(another.runtime(), another.is_runtime_separated()); + new_function.set_runtime(another.runtime()); new_function } @@ -117,14 +117,9 @@ impl Function { self.dfg.runtime() } - /// Indicate whether the runtimes have been separated yet. - pub(crate) fn is_runtime_separated(&self) -> bool { - self.dfg.is_runtime_separated() - } - /// Set runtime type of the function. - pub(crate) fn set_runtime(&mut self, runtime: RuntimeType, separated: bool) { - self.dfg.set_runtime(runtime, separated); + pub(crate) fn set_runtime(&mut self, runtime: RuntimeType) { + self.dfg.set_runtime(runtime); } pub(crate) fn is_no_predicates(&self) -> bool { @@ -198,37 +193,6 @@ impl Function { unreachable!("SSA Function {} has no reachable return instruction!", self.id()) } - - /// Remove instructions from all the reachable blocks in a function based on a predicate, - /// keeping the ones where the predicate returns `true`. - pub(crate) fn retain_instructions(&mut self, pred: impl Fn(&Instruction) -> bool) { - for block_id in self.reachable_blocks() { - let mut to_remove = HashSet::new(); - let block = &self.dfg[block_id]; - for instruction_id in block.instructions() { - let instruction = &self.dfg[*instruction_id]; - if !pred(instruction) { - to_remove.insert(*instruction_id); - } - } - if !to_remove.is_empty() { - let block = &mut self.dfg[block_id]; - block.instructions_mut().retain(|id| !to_remove.contains(id)); - } - } - } - - /// Remove all instructions that aren't handled by the runtime, which is now - /// considered final. Seal the [DataFlowGraph] so it instantly simplifies - /// away instructions that the runtime would have to ignore. - /// - /// TODO(#6841): Remove once the SSA generated is for a specific runtime already. - pub(crate) fn separate_runtime(&mut self) { - if self.runtime().is_acir() { - self.retain_instructions(|i| !i.is_brillig_only()); - } - self.dfg.set_runtime_separated(); - } } impl Clone for Function { diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 506e7578bb..1f6c7433f8 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -361,7 +361,7 @@ impl InlineContext { ) -> InlineContext { let source = &ssa.functions[&entry_point]; let mut builder = FunctionBuilder::new(source.name().to_owned(), entry_point); - builder.set_runtime(source.runtime(), source.is_runtime_separated()); + builder.set_runtime(source.runtime()); Self { builder, recursion_level: 0, diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 58f40845e9..e89d1d2a0c 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -107,7 +107,7 @@ impl<'a> FunctionContext<'a> { .1; let mut builder = FunctionBuilder::new(function_name, function_id); - builder.set_runtime(runtime, false); + builder.set_runtime(runtime); let definitions = HashMap::default(); let mut this = Self { definitions, builder, shared_context, loops: Vec::new() }; this.add_parameters_to_scope(parameters); From eebf6502f93d0101aa42a1f62ab174eee524cd85 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 6 Jan 2025 19:30:35 +0000 Subject: [PATCH 07/11] . --- .../src/brillig/brillig_gen/brillig_slice_ops.rs | 2 +- .../src/brillig/brillig_gen/variable_liveness.rs | 6 +++--- compiler/noirc_evaluator/src/ssa/opt/inlining.rs | 4 ++-- compiler/noirc_evaluator/src/ssa/opt/rc.rs | 2 +- compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs | 3 +-- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs index 320cac602c..26c7151bf0 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_slice_ops.rs @@ -180,7 +180,7 @@ mod tests { fn create_test_environment() -> (Ssa, FunctionContext, BrilligContext) { let mut builder = FunctionBuilder::new("main".to_string(), Id::test_new(0)); - builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let ssa = builder.finish(); let mut brillig_context = create_context(ssa.main_id); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs index 6884c9a67c..d6851a9ecf 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_gen/variable_liveness.rs @@ -361,7 +361,7 @@ mod test { let main_id = Id::test_new(1); let mut builder = FunctionBuilder::new("main".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let b1 = builder.insert_block(); let b2 = builder.insert_block(); @@ -471,7 +471,7 @@ mod test { let main_id = Id::test_new(1); let mut builder = FunctionBuilder::new("main".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let b1 = builder.insert_block(); let b2 = builder.insert_block(); @@ -610,7 +610,7 @@ mod test { let main_id = Id::test_new(1); let mut builder = FunctionBuilder::new("main".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let v0 = builder.add_parameter(Type::bool()); diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index 1f6c7433f8..3695548072 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -1164,7 +1164,7 @@ mod test { // } let foo_id = Id::test_new(0); let mut builder = FunctionBuilder::new("foo".into(), foo_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default()), true); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let bar_id = Id::test_new(1); let bar = builder.import_function(bar_id); @@ -1206,7 +1206,7 @@ mod test { // } let foo_id = Id::test_new(0); let mut builder = FunctionBuilder::new("foo".into(), foo_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default()), true); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let bar_id = Id::test_new(1); let bar = builder.import_function(bar_id); diff --git a/compiler/noirc_evaluator/src/ssa/opt/rc.rs b/compiler/noirc_evaluator/src/ssa/opt/rc.rs index 18bf36f3f4..64f6e2ddfe 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/rc.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/rc.rs @@ -206,7 +206,7 @@ mod test { // } let main_id = Id::test_new(0); let mut builder = FunctionBuilder::new("foo".into(), main_id); - builder.set_runtime(RuntimeType::Brillig(InlineType::default()), false); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let inner_array_type = Type::Array(Arc::new(vec![Type::field()]), 2); let v0 = builder.add_parameter(inner_array_type.clone()); diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 29b2350326..fcaaf74f53 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -59,8 +59,7 @@ impl Translator { let main_function = parsed_ssa.functions.remove(0); let main_id = FunctionId::test_new(0); let mut builder = FunctionBuilder::new(main_function.external_name.clone(), main_id); - let separated = false; // Allow the builder to do anything it wants. - builder.set_runtime(main_function.runtime_type, separated); + builder.set_runtime(main_function.runtime_type); builder.simplify = simplify; // Map function names to their IDs so calls can be resolved From 694e27cb3d08dc68095f037e6faf2eb6a32941d5 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 6 Jan 2025 19:45:42 +0000 Subject: [PATCH 08/11] Update compiler/noirc_evaluator/src/ssa/ir/dfg.rs Co-authored-by: Maxim Vezenov --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 83f81a038a..0379f852cf 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -108,18 +108,7 @@ impl DataFlowGraph { pub(crate) fn new(runtime: RuntimeType) -> Self { Self { runtime, - instructions: Default::default(), - results: Default::default(), - values: Default::default(), - constants: Default::default(), - functions: Default::default(), - intrinsics: Default::default(), - foreign_functions: Default::default(), - blocks: Default::default(), - replaced_value_ids: Default::default(), - locations: Default::default(), - call_stack_data: Default::default(), - data_bus: Default::default(), + ..Default::default() } } From bdb3e91157e8e1de9d6f11e7f180c3d43f0a03ac Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 6 Jan 2025 19:50:17 +0000 Subject: [PATCH 09/11] Revert "Update compiler/noirc_evaluator/src/ssa/ir/dfg.rs" This reverts commit 694e27cb3d08dc68095f037e6faf2eb6a32941d5. --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 0379f852cf..83f81a038a 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -108,7 +108,18 @@ impl DataFlowGraph { pub(crate) fn new(runtime: RuntimeType) -> Self { Self { runtime, - ..Default::default() + instructions: Default::default(), + results: Default::default(), + values: Default::default(), + constants: Default::default(), + functions: Default::default(), + intrinsics: Default::default(), + foreign_functions: Default::default(), + blocks: Default::default(), + replaced_value_ids: Default::default(), + locations: Default::default(), + call_stack_data: Default::default(), + data_bus: Default::default(), } } From a7052cab65d3deb96a7fbd467f8000d03fcb18ee Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 6 Jan 2025 20:11:43 +0000 Subject: [PATCH 10/11] Set runtime to Brillig in tests which expect RC instructions to be added --- compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs | 4 ++++ compiler/noirc_evaluator/src/ssa/opt/rc.rs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 3a33accf3d..17aefdf21b 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -830,9 +830,12 @@ fn simplify(dfg: &DataFlowGraph, lhs: ValueId, rhs: ValueId) -> Option<(ValueId, mod test { use std::sync::Arc; + use noirc_frontend::monomorphization::ast::InlineType; + use crate::ssa::{ function_builder::FunctionBuilder, ir::{ + function::RuntimeType, map::Id, types::{NumericType, Type}, }, @@ -1153,6 +1156,7 @@ mod test { // Compiling main let mut builder = FunctionBuilder::new("main".into(), main_id); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let v0 = builder.add_parameter(Type::unsigned(64)); let zero = builder.numeric_constant(0u128, NumericType::unsigned(64)); let typ = Type::Array(Arc::new(vec![Type::unsigned(64)]), 25); diff --git a/compiler/noirc_evaluator/src/ssa/opt/rc.rs b/compiler/noirc_evaluator/src/ssa/opt/rc.rs index 64f6e2ddfe..e25ad35014 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/rc.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/rc.rs @@ -246,6 +246,7 @@ mod test { // } let main_id = Id::test_new(0); let mut builder = FunctionBuilder::new("mutator".into(), main_id); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let array_type = Type::Array(Arc::new(vec![Type::field()]), 2); let v0 = builder.add_parameter(array_type.clone()); @@ -295,6 +296,7 @@ mod test { // } let main_id = Id::test_new(0); let mut builder = FunctionBuilder::new("mutator2".into(), main_id); + builder.set_runtime(RuntimeType::Brillig(InlineType::default())); let array_type = Type::Array(Arc::new(vec![Type::field()]), 2); let reference_type = Type::Reference(Arc::new(array_type.clone())); From d47947d70a495b7b3b58cc821d51e36658b25bc9 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Mon, 6 Jan 2025 20:16:56 +0000 Subject: [PATCH 11/11] Add Default for RuntimeType --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 26 +++---------------- .../noirc_evaluator/src/ssa/ir/function.rs | 8 +++++- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 83f81a038a..8f1b360a12 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -26,7 +26,7 @@ use serde_with::DisplayFromStr; /// owning most data in a function and handing out Ids to this data that can be /// shared without worrying about ownership. #[serde_as] -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub(crate) struct DataFlowGraph { /// Runtime of the [Function] that owns this [DataFlowGraph]. /// This might change during the `runtime_separation` pass where @@ -105,24 +105,6 @@ pub(crate) struct DataFlowGraph { } impl DataFlowGraph { - pub(crate) fn new(runtime: RuntimeType) -> Self { - Self { - runtime, - instructions: Default::default(), - results: Default::default(), - values: Default::default(), - constants: Default::default(), - functions: Default::default(), - intrinsics: Default::default(), - foreign_functions: Default::default(), - blocks: Default::default(), - replaced_value_ids: Default::default(), - locations: Default::default(), - call_stack_data: Default::default(), - data_bus: Default::default(), - } - } - /// Runtime type of the function. pub(crate) fn runtime(&self) -> RuntimeType { self.runtime @@ -725,14 +707,12 @@ impl<'dfg> std::ops::Index for InsertInstructionResult<'dfg> { #[cfg(test)] mod tests { - use noirc_frontend::monomorphization::ast::InlineType; - use super::DataFlowGraph; - use crate::ssa::ir::{dfg::RuntimeType, instruction::Instruction, types::Type}; + use crate::ssa::ir::{instruction::Instruction, types::Type}; #[test] fn make_instruction() { - let mut dfg = DataFlowGraph::new(RuntimeType::Acir(InlineType::Inline)); + let mut dfg = DataFlowGraph::default(); let ins = Instruction::Allocate; let ins_id = dfg.make_instruction(ins, Some(vec![Type::field()])); diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 25c3573fbd..109c2a5978 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -56,6 +56,12 @@ impl RuntimeType { } } +impl Default for RuntimeType { + fn default() -> Self { + RuntimeType::Acir(InlineType::default()) + } +} + /// A function holds a list of instructions. /// These instructions are further grouped into Basic blocks /// @@ -82,7 +88,7 @@ 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::new(RuntimeType::Acir(InlineType::default())); + let mut dfg = DataFlowGraph::default(); let entry_block = dfg.make_block(); Self { name, id, entry_block, dfg } }