From 280628f9854680234d7c827f88c849d5b112b7a3 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 12 Jun 2023 09:52:35 -0500 Subject: [PATCH 1/3] Expand PR --- .../src/ssa_refactor/ir/dfg.rs | 22 +++++++++---------- .../src/ssa_refactor/ir/printer.rs | 5 ++--- .../src/ssa_refactor/opt/constant_folding.rs | 2 +- .../src/ssa_refactor/opt/flatten_cfg.rs | 3 ++- .../src/ssa_refactor/opt/inlining.rs | 5 +++-- .../src/ssa_refactor/opt/unrolling.rs | 3 ++- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index ebeecc5cb4e..cff4b3d32ca 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -157,24 +157,22 @@ impl DataFlowGraph { /// values since other instructions referring to the same ValueId need /// not be modified to refer to a new ValueId. pub(crate) fn set_value_from_id(&mut self, value_to_replace: ValueId, new_value: ValueId) { - // Replaced `ValueId`s are tracked purely for debugging purposes. - // We first call `resolve_replaced_value_id(new_value)` in case `new_value` is also a - // `ValueId` that has previously had its corresponding `Value` substituted. - self.replaced_value_ids.insert(value_to_replace, self.resolve_replaced_value_id(new_value)); - - let new_value = self.values[new_value].clone(); - self.values[value_to_replace] = new_value; + if value_to_replace != new_value { + self.replaced_value_ids.insert(value_to_replace, self.resolve(new_value)); + let new_value = self.values[new_value].clone(); + self.values[value_to_replace] = new_value; + } } /// If `original_value_id`'s underlying `Value` has been substituted for that of another /// `ValueId`, this function will return the `ValueId` from which the substitution was taken. /// If `original_value_id`'s underlying `Value` has not been substituted, the same `ValueId` /// is returned. - /// - /// The function exists purely for debugging purposes. Only the SSA printer is concerned with - /// this information. - pub(crate) fn resolve_replaced_value_id(&self, original_value_id: ValueId) -> ValueId { - *self.replaced_value_ids.get(&original_value_id).unwrap_or(&original_value_id) + pub(crate) fn resolve(&self, original_value_id: ValueId) -> ValueId { + match self.replaced_value_ids.get(&original_value_id) { + Some(id) => self.resolve(*id), + None => original_value_id, + } } /// Creates a new constant value, or returns the Id to an existing one if diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs index fc4681085f4..16a35d19798 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/printer.rs @@ -61,6 +61,7 @@ pub(crate) fn display_block( /// constant or a function we print those directly. fn value(function: &Function, id: ValueId) -> String { use super::value::Value; + let id = function.dfg.resolve(id); match &function.dfg[id] { Value::NumericConstant { constant, typ } => { format!("{typ} {constant}") @@ -71,9 +72,7 @@ fn value(function: &Function, id: ValueId) -> String { let elements = vecmap(array, |element| value(function, *element)); format!("[{}]", elements.join(", ")) } - Value::Param { .. } | Value::Instruction { .. } => { - function.dfg.resolve_replaced_value_id(id).to_string() - } + Value::Param { .. } | Value::Instruction { .. } => id.to_string(), } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs index 5a36ab799c7..6b3b57a4ffa 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs @@ -75,7 +75,7 @@ impl Context { id: InstructionId, ) { let instruction = function.dfg[id].map_values(|id| self.get_value(id)); - let results = function.dfg.instruction_results(id).to_vec(); + let results = vecmap(function.dfg.instruction_results(id), |id| function.dfg.resolve(*id)); let ctrl_typevars = instruction .requires_ctrl_typevars() diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs index 313400e8438..29c055777ba 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs @@ -534,7 +534,8 @@ impl<'f> Context<'f> { fn push_instruction(&mut self, id: InstructionId) { let instruction = self.function.dfg[id].map_values(|id| self.translate_value(id)); let instruction = self.handle_instruction_side_effects(instruction); - let results = self.function.dfg.instruction_results(id).to_vec(); + let results = self.function.dfg.instruction_results(id); + let results = vecmap(results, |id| self.function.dfg.resolve(*id)); let ctrl_typevars = instruction .requires_ctrl_typevars() diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index a403ef9b161..d7fc8b78f61 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -362,13 +362,14 @@ impl<'function> PerFunctionContext<'function> { fn push_instruction(&mut self, id: InstructionId) { let instruction = self.source_function.dfg[id].map_values(|id| self.translate_value(id)); let results = self.source_function.dfg.instruction_results(id); + let results = vecmap(results, |id| self.source_function.dfg.resolve(*id)); let ctrl_typevars = instruction .requires_ctrl_typevars() - .then(|| vecmap(results, |result| self.source_function.dfg.type_of_value(*result))); + .then(|| vecmap(&results, |result| self.source_function.dfg.type_of_value(*result))); let new_results = self.context.builder.insert_instruction(instruction, ctrl_typevars); - Self::insert_new_instruction_results(&mut self.values, results, new_results); + Self::insert_new_instruction_results(&mut self.values, &results, new_results); } /// Modify the values HashMap to remember the mapping between an instruction result's previous diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/unrolling.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/unrolling.rs index 72026ed81ae..b420b4ca4f0 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/unrolling.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/unrolling.rs @@ -441,7 +441,8 @@ impl<'f> LoopIteration<'f> { fn push_instruction(&mut self, id: InstructionId) { let instruction = self.function.dfg[id].map_values(|id| self.get_value(id)); - let results = self.function.dfg.instruction_results(id).to_vec(); + let results = self.function.dfg.instruction_results(id); + let results = vecmap(results, |id| self.function.dfg.resolve(*id)); let ctrl_typevars = instruction .requires_ctrl_typevars() From 2389d48c089968f98bda6a08ddc5734a969de57c Mon Sep 17 00:00:00 2001 From: Joss Date: Mon, 12 Jun 2023 17:00:28 +0100 Subject: [PATCH 2/3] chore(ssa refactor): more value id resolving --- crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs | 1 + .../src/ssa_refactor/opt/constant_folding.rs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 4148ee31082..2cf6125b788 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -325,6 +325,7 @@ impl Context { /// involving such values are evaluated via a separate path and stored in /// `ssa_value_to_array_address` instead. fn convert_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> AcirValue { + let value_id = dfg.resolve(value_id); let value = &dfg[value_id]; if let Some(acir_value) = self.ssa_values.get(&value_id) { return acir_value.clone(); diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs index 6b3b57a4ffa..4395aeae583 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs @@ -56,9 +56,9 @@ impl Context { for instruction in instructions { self.push_instruction(function, block, instruction); } - - let terminator = - function.dfg[block].unwrap_terminator().map_values(|value| self.get_value(value)); + let terminator = function.dfg[block] + .unwrap_terminator() + .map_values(|value| self.get_value(function.dfg.resolve(value))); function.dfg.set_block_terminator(block, terminator); self.block_queue.extend(function.dfg[block].successors()); From ab3a8254a226ae45ff677cef248c731609d22582 Mon Sep 17 00:00:00 2001 From: Joss Date: Mon, 12 Jun 2023 18:40:21 +0100 Subject: [PATCH 3/3] chore(ssa refactor): another value id resolve --- .../noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs index 4395aeae583..8b0db177500 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/constant_folding.rs @@ -74,7 +74,8 @@ impl Context { block: BasicBlockId, id: InstructionId, ) { - let instruction = function.dfg[id].map_values(|id| self.get_value(id)); + let instruction = + function.dfg[id].map_values(|id| self.get_value(function.dfg.resolve(id))); let results = vecmap(function.dfg.instruction_results(id), |id| function.dfg.resolve(*id)); let ctrl_typevars = instruction