From 959ee739f61e48ab920d29f481ce185ab8304079 Mon Sep 17 00:00:00 2001 From: Tom French Date: Fri, 1 Sep 2023 23:54:33 +0100 Subject: [PATCH 1/8] chore: refactor constant folding pass --- .../src/ssa/opt/constant_folding.rs | 95 ++++++++++++------- 1 file changed, 62 insertions(+), 33 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index 295353989b2..e7af88eabd1 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -5,7 +5,7 @@ use iter_extended::vecmap; use crate::ssa::{ ir::{ basic_block::BasicBlockId, - dfg::InsertInstructionResult, + dfg::{DataFlowGraph, InsertInstructionResult}, function::Function, instruction::{Instruction, InstructionId}, value::ValueId, @@ -14,7 +14,12 @@ use crate::ssa::{ }; impl Ssa { - /// Performs constant folding on each instruction. + /// Performs constant folding on each instruction. This is done by two methods: + /// + /// 1. Re-insert each instruction in order to apply the constant folding which is done automatically + /// by the [`DataFlowGraph`] as new instructions are pushed. + /// 2. Check for the existence of [pure instructions][Instruction::is_pure()] which have a duplicate earlier in the block. + /// These can be replaced with the results of this previous instruction. /// /// This is generally done automatically but this pass can become needed /// if `DataFlowGraph::set_value` or `DataFlowGraph::set_value_from_id` are @@ -59,59 +64,83 @@ impl Context { let mut cached_instruction_results: HashMap> = HashMap::new(); for instruction_id in instructions { - self.push_instruction(function, block, instruction_id, &mut cached_instruction_results); + Self::fold_constants_into_instruction( + &mut function.dfg, + block, + instruction_id, + &mut cached_instruction_results, + ); } self.block_queue.extend(function.dfg[block].successors()); } - fn push_instruction( - &mut self, - function: &mut Function, + fn fold_constants_into_instruction( + dfg: &mut DataFlowGraph, block: BasicBlockId, id: InstructionId, instruction_result_cache: &mut HashMap>, ) { - let instruction = function.dfg[id].clone(); - let old_results = function.dfg.instruction_results(id).to_vec(); - - // Resolve any inputs to ensure that we're comparing like-for-like instructions. - let instruction = instruction.map_values(|value_id| function.dfg.resolve(value_id)); + let instruction = Self::resolve_instruction(id, dfg); + let old_results = dfg.instruction_results(id).to_vec(); - // If a copy of this instruction exists earlier in the block then reuse the previous results. + // If a copy of this instruction exists earlier in the block, then reuse the previous results. if let Some(cached_results) = instruction_result_cache.get(&instruction) { for (old_result, new_result) in old_results.iter().zip(cached_results) { - function.dfg.set_value_from_id(*old_result, *new_result); + dfg.set_value_from_id(*old_result, *new_result); } return; } - let ctrl_typevars = instruction - .requires_ctrl_typevars() - .then(|| vecmap(&old_results, |result| function.dfg.type_of_value(*result))); - - let call_stack = function.dfg.get_call_stack(id); - let new_results = match function.dfg.insert_instruction_and_results( - instruction.clone(), - block, - ctrl_typevars, - call_stack, - ) { - InsertInstructionResult::SimplifiedTo(new_result) => vec![new_result], - InsertInstructionResult::SimplifiedToMultiple(new_results) => new_results, - InsertInstructionResult::Results(_, new_results) => new_results.to_vec(), - InsertInstructionResult::InstructionRemoved => vec![], - }; - assert_eq!(old_results.len(), new_results.len()); + // Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs. + let new_results = Self::push_instruction(id, instruction.clone(), &old_results, block, dfg); - // If the instruction doesn't have side-effects, cache the results so we can reuse them if + // If the instruction is pure then we cache the results so we can reuse them if // the same instruction appears again later in the block. - if instruction.is_pure(&function.dfg) { + if instruction.is_pure(dfg) { instruction_result_cache.insert(instruction, new_results.clone()); } for (old_result, new_result) in old_results.iter().zip(new_results) { - function.dfg.set_value_from_id(*old_result, new_result); + dfg.set_value_from_id(*old_result, new_result); } } + + /// Fetches an [`Instruction`] by its [`InstructionId`] and fully resolves its inputs. + fn resolve_instruction(instruction_id: InstructionId, dfg: &DataFlowGraph) -> Instruction { + let instruction = dfg[instruction_id].clone(); + + // Resolve any inputs to ensure that we're comparing like-for-like instructions. + instruction.map_values(|value_id| dfg.resolve(value_id)) + } + + /// Pushes a new [`Instruction`] into the [`DataFlowGraph`] which applies any optimizations + /// based on newly resolved values for its inputs. + /// + /// This may result in the [`Instruction`] being optimized away or replaced with a constant value. + fn push_instruction( + id: InstructionId, + instruction: Instruction, + old_results: &[ValueId], + block: BasicBlockId, + dfg: &mut DataFlowGraph, + ) -> Vec { + let ctrl_typevars = instruction + .requires_ctrl_typevars() + .then(|| vecmap(old_results, |result| dfg.type_of_value(*result))); + + let call_stack = dfg.get_call_stack(id); + let new_results = + match dfg.insert_instruction_and_results(instruction, block, ctrl_typevars, call_stack) + { + InsertInstructionResult::SimplifiedTo(new_result) => vec![new_result], + InsertInstructionResult::SimplifiedToMultiple(new_results) => new_results, + InsertInstructionResult::Results(_, new_results) => new_results.to_vec(), + InsertInstructionResult::InstructionRemoved => vec![], + }; + // Optimizations while inserting the instruction should not change the number of results. + assert_eq!(old_results.len(), new_results.len()); + + new_results + } } #[cfg(test)] From 07fcf356e09ce533a8e1442ad090c4323175cdfd Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 2 Sep 2023 00:28:54 +0100 Subject: [PATCH 2/8] chore: restructure documentation --- .../noirc_evaluator/src/ssa/opt/constant_folding.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index e7af88eabd1..ddc81b70240 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -18,13 +18,17 @@ impl Ssa { /// /// 1. Re-insert each instruction in order to apply the constant folding which is done automatically /// by the [`DataFlowGraph`] as new instructions are pushed. + /// + /// This is generally done automatically but this pass can become needed + /// if `DataFlowGraph::set_value` or `DataFlowGraph::set_value_from_id` are + /// used on a value which enables instructions dependent on the value to + /// now be simplified. + /// /// 2. Check for the existence of [pure instructions][Instruction::is_pure()] which have a duplicate earlier in the block. /// These can be replaced with the results of this previous instruction. /// - /// This is generally done automatically but this pass can become needed - /// if `DataFlowGraph::set_value` or `DataFlowGraph::set_value_from_id` are - /// used on a value which enables instructions dependent on the value to - /// now be simplified. + /// This is only performed within this pass and so is needed when different blocks are merged, + /// i.e. after the [`flatten_cfg`][super::flatten_cfg] pass. pub(crate) fn fold_constants(mut self) -> Ssa { for function in self.functions.values_mut() { constant_fold(function); From 7dd8a07176bba8625b4497759aaa20dd644d4bc1 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 2 Sep 2023 01:19:21 +0100 Subject: [PATCH 3/8] chore: add test for instruction deduplication --- .../src/ssa/opt/constant_folding.rs | 49 ++++++++++++++++++- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index ddc81b70240..83d9e9162ed 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -155,10 +155,10 @@ mod test { function_builder::FunctionBuilder, ir::{ function::RuntimeType, - instruction::{BinaryOp, TerminatorInstruction}, + instruction::{BinaryOp, Instruction, TerminatorInstruction}, map::Id, types::Type, - value::Value, + value::{Value, ValueId}, }, }; @@ -260,4 +260,49 @@ mod test { // The return element is expected to refer to the new add instruction result. assert_eq!(main.dfg.resolve(new_add_instr_result), main.dfg.resolve(return_element)); } + + #[test] + fn instruction_deduplication() { + // fn main f0 { + // b0(v0: Field): + // v1 = cast v0 as u32 + // v2 = cast v0 as u32 + // constrain v1 v2 + // } + // + // After constructing this IR, we run constant folding which should replace the second cast + // with a reference to the outputs of the results to the first. This then allows us to optimize away + // the constrain instruction as both inputs are known to be equal. + // + // The first cast instruction is retained and will be removed in the dead instruction elimination pass. + let main_id = Id::test_new(0); + + // Compiling main + let mut builder = FunctionBuilder::new("main".into(), main_id, RuntimeType::Acir); + let v0 = builder.add_parameter(Type::field()); + + let v1 = builder.insert_cast(v0, Type::unsigned(32)); + let v2 = builder.insert_cast(v0, Type::unsigned(32)); + builder.insert_constrain(v1, v2); + + let mut ssa = builder.finish(); + let main = ssa.main_mut(); + let instructions = main.dfg[main.entry_block()].instructions(); + assert_eq!(instructions.len(), 3); + + // Expected output: + // + // fn main f0 { + // b0(v0: Field): + // v1 = cast v0 as u32 + // } + let ssa = ssa.fold_constants(); + let main = ssa.main(); + let instructions = main.dfg[main.entry_block()].instructions(); + + assert_eq!(instructions.len(), 1); + let instruction = &main.dfg[instructions[0]]; + + assert_eq!(instruction, &Instruction::Cast(ValueId::test_new(0), Type::unsigned(32))); + } } From 848907ad63ad75db6ae80c370f6f9b642caae22b Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 2 Sep 2023 01:20:21 +0100 Subject: [PATCH 4/8] chore: typo --- crates/noirc_evaluator/src/ssa/opt/constant_folding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index 83d9e9162ed..9c0579b3a2c 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -271,7 +271,7 @@ mod test { // } // // After constructing this IR, we run constant folding which should replace the second cast - // with a reference to the outputs of the results to the first. This then allows us to optimize away + // with a reference to the results to the first. This then allows us to optimize away // the constrain instruction as both inputs are known to be equal. // // The first cast instruction is retained and will be removed in the dead instruction elimination pass. From e160f846af88b06fc101db506e9854cd7d92b8e3 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sat, 2 Sep 2023 01:53:16 +0100 Subject: [PATCH 5/8] chore: factor out a function to replace a set of `ValueId`s --- .../src/ssa/opt/constant_folding.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index 9c0579b3a2c..65700460b24 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -89,9 +89,7 @@ impl Context { // If a copy of this instruction exists earlier in the block, then reuse the previous results. if let Some(cached_results) = instruction_result_cache.get(&instruction) { - for (old_result, new_result) in old_results.iter().zip(cached_results) { - dfg.set_value_from_id(*old_result, *new_result); - } + Self::replace_result_ids(dfg, &old_results, cached_results); return; } @@ -103,9 +101,7 @@ impl Context { if instruction.is_pure(dfg) { instruction_result_cache.insert(instruction, new_results.clone()); } - for (old_result, new_result) in old_results.iter().zip(new_results) { - dfg.set_value_from_id(*old_result, new_result); - } + Self::replace_result_ids(dfg, &old_results, &new_results); } /// Fetches an [`Instruction`] by its [`InstructionId`] and fully resolves its inputs. @@ -145,6 +141,17 @@ impl Context { new_results } + + /// Replaces a set of [`ValueId`]s inside the [`DataFlowGraph`] with another. + fn replace_result_ids( + dfg: &mut DataFlowGraph, + old_results: &[ValueId], + new_results: &[ValueId], + ) { + for (old_result, new_result) in old_results.iter().zip(new_results) { + dfg.set_value_from_id(*old_result, *new_result); + } + } } #[cfg(test)] From 5b94612e52e48503599ccf80ab1f586175edd065 Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 3 Sep 2023 01:26:19 +0100 Subject: [PATCH 6/8] chore: rework documentation for `constant_folding` pass --- .../src/ssa/opt/constant_folding.rs | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index 65700460b24..25717b78cd3 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -1,3 +1,23 @@ +//! The goal of the constant folding optimization pass is to propagate any constants forwards into +//! later [`Instruction`]s to maximise the impact of [compile-time simplifications][Instruction::simplify()]. +//! +//! The pass works as follows: +//! - Re-insert each instruction in order to apply the instruction simplification performed +//! by the [`DataFlowGraph`] automatically as new instructions are pushed. +//! - Check whether the instruction is [pure][Instruction::is_pure()] +//! and there exists a duplicate instruction earlier in the same block. +//! If so, the instruction can be replaced with the results of this previous instruction. +//! +//! These operations are done in parallel so that they can each benefit from each other +//! without the need for multiple passes. +//! +//! Other passes perform a certain amount of constant folding automatically as they insert instructions +//! into the [`DataFlowGraph`] but this pass can become needed if [`DataFlowGraph::set_value`] or +//! [`DataFlowGraph::set_value_from_id`] are used on a value which enables instructions dependent on the value to +//! now be simplified. +//! +//! This is the only pass which removes duplicated pure [`Instruction`]s however and so is needed when +//! different blocks are merged, i.e. after the [`flatten_cfg`][super::flatten_cfg] pass. use std::collections::{HashMap, HashSet}; use iter_extended::vecmap; @@ -14,21 +34,9 @@ use crate::ssa::{ }; impl Ssa { - /// Performs constant folding on each instruction. This is done by two methods: + /// Performs constant folding on each instruction. /// - /// 1. Re-insert each instruction in order to apply the constant folding which is done automatically - /// by the [`DataFlowGraph`] as new instructions are pushed. - /// - /// This is generally done automatically but this pass can become needed - /// if `DataFlowGraph::set_value` or `DataFlowGraph::set_value_from_id` are - /// used on a value which enables instructions dependent on the value to - /// now be simplified. - /// - /// 2. Check for the existence of [pure instructions][Instruction::is_pure()] which have a duplicate earlier in the block. - /// These can be replaced with the results of this previous instruction. - /// - /// This is only performed within this pass and so is needed when different blocks are merged, - /// i.e. after the [`flatten_cfg`][super::flatten_cfg] pass. + /// See [`constant_folding`][self] module for more infomation. pub(crate) fn fold_constants(mut self) -> Ssa { for function in self.functions.values_mut() { constant_fold(function); From 2ba69e85a5a5c882cfa54141d1ed95c294b39b11 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 4 Sep 2023 10:55:56 +0100 Subject: [PATCH 7/8] Apply suggestions from code review --- crates/noirc_evaluator/src/ssa/opt/constant_folding.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index 25717b78cd3..a7c0f790614 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -1,5 +1,5 @@ //! The goal of the constant folding optimization pass is to propagate any constants forwards into -//! later [`Instruction`]s to maximise the impact of [compile-time simplifications][Instruction::simplify()]. +//! later [`Instruction`]s to maximize the impact of [compile-time simplifications][Instruction::simplify()]. //! //! The pass works as follows: //! - Re-insert each instruction in order to apply the instruction simplification performed @@ -36,7 +36,7 @@ use crate::ssa::{ impl Ssa { /// Performs constant folding on each instruction. /// - /// See [`constant_folding`][self] module for more infomation. + /// See [`constant_folding`][self] module for more information. pub(crate) fn fold_constants(mut self) -> Ssa { for function in self.functions.values_mut() { constant_fold(function); From d7c7cd31dd3cda06c98419b782f4ca53795f194d Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 5 Sep 2023 17:54:35 +0100 Subject: [PATCH 8/8] chore: fix error from merge --- crates/noirc_evaluator/src/ssa/opt/constant_folding.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs index 1a34fccdfdc..51592a13ae5 100644 --- a/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -299,7 +299,7 @@ mod test { let v1 = builder.insert_cast(v0, Type::unsigned(32)); let v2 = builder.insert_cast(v0, Type::unsigned(32)); - builder.insert_constrain(v1, v2); + builder.insert_constrain(v1, v2, None); let mut ssa = builder.finish(); let main = ssa.main_mut();