From e61c3882f3e3bf3f12f2bd4566aa747c5b0af2bb Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 9 Dec 2024 10:07:38 -0600 Subject: [PATCH 1/2] Unrolling cleanup --- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 68 +++++++------------ 1 file changed, 26 insertions(+), 42 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index 22daba1de45..b5b1155b0b4 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -279,10 +279,10 @@ impl Loop { &self, function: &Function, cfg: &ControlFlowGraph, - ) -> Result, CallStack> { - let pre_header = self.get_pre_header(function, cfg)?; - let jump_value = get_induction_variable(function, pre_header)?; - Ok(function.dfg.get_numeric_constant(jump_value)) + ) -> Option { + let pre_header = self.get_pre_header(function, cfg).ok()?; + let jump_value = get_induction_variable(function, pre_header).ok()?; + function.dfg.get_numeric_constant(jump_value) } /// Find the upper bound of the loop in the loop header and return it @@ -327,14 +327,10 @@ impl Loop { &self, function: &Function, cfg: &ControlFlowGraph, - ) -> Result, CallStack> { - let Some(lower) = self.get_const_lower_bound(function, cfg)? else { - return Ok(None); - }; - let Some(upper) = self.get_const_upper_bound(function) else { - return Ok(None); - }; - Ok(Some((lower, upper))) + ) -> Option<(FieldElement, FieldElement)> { + let lower = self.get_const_lower_bound(function, cfg)?; + let upper = self.get_const_upper_bound(function)?; + Some((lower, upper)) } /// Unroll a single loop in the function. @@ -547,9 +543,9 @@ impl Loop { &self, function: &Function, cfg: &ControlFlowGraph, - ) -> Result, CallStack> { + ) -> Option> { // We need to traverse blocks from the pre-header up to the block entry point. - let pre_header = self.get_pre_header(function, cfg)?; + let pre_header = self.get_pre_header(function, cfg).ok()?; let function_entry = function.entry_block(); // The algorithm in `find_blocks_in_loop` expects to collect the blocks between the header and the back-edge of the loop, @@ -557,22 +553,19 @@ impl Loop { let blocks = Self::find_blocks_in_loop(function_entry, pre_header, cfg).blocks; // Collect allocations in all blocks above the header. - let allocations = blocks.iter().flat_map(|b| { - function.dfg[*b] - .instructions() - .iter() + let allocations = blocks.iter().flat_map(|block| { + let instructions = function.dfg[*block].instructions().iter(); + instructions .filter(|i| matches!(&function.dfg[**i], Instruction::Allocate)) - .map(|i| { - // Get the value into which the allocation was stored. - function.dfg.instruction_results(*i)[0] - }) + // Get the value into which the allocation was stored. + .map(|i| function.dfg.instruction_results(*i)[0]) }); // Collect reference parameters of the function itself. let params = function.parameters().iter().filter(|p| function.dfg.value_is_reference(**p)).copied(); - Ok(params.chain(allocations).collect()) + Some(params.chain(allocations).collect()) } /// Count the number of load and store instructions of specific variables in the loop. @@ -603,13 +596,11 @@ impl Loop { /// Count the number of instructions in the loop, including the terminating jumps. fn count_all_instructions(&self, function: &Function) -> usize { - self.blocks - .iter() - .map(|block| { - let block = &function.dfg[*block]; - block.instructions().len() + block.terminator().map(|_| 1).unwrap_or_default() - }) - .sum() + let iter = self.blocks.iter().map(|block| { + let block = &function.dfg[*block]; + block.instructions().len() + block.terminator().is_some() as usize + }); + iter.sum() } /// Count the number of increments to the induction variable. @@ -640,18 +631,11 @@ impl Loop { function: &Function, cfg: &ControlFlowGraph, ) -> Option { - let Ok(Some((lower, upper))) = self.get_const_bounds(function, cfg) else { - return None; - }; - let Some(lower) = lower.try_to_u64() else { - return None; - }; - let Some(upper) = upper.try_to_u64() else { - return None; - }; - let Ok(refs) = self.find_pre_header_reference_values(function, cfg) else { - return None; - }; + let (lower, upper) = self.get_const_bounds(function, cfg)?; + let lower = lower.try_to_u64()?; + let upper = upper.try_to_u64()?; + let refs = self.find_pre_header_reference_values(function, cfg)?; + let (loads, stores) = self.count_loads_and_stores(function, &refs); let increments = self.count_induction_increments(function); let all_instructions = self.count_all_instructions(function); From b3e080ed7568c16de1cbd5f4758638b10f6409ee Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 9 Dec 2024 10:09:26 -0600 Subject: [PATCH 2/2] Update test --- compiler/noirc_evaluator/src/ssa/opt/unrolling.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index b5b1155b0b4..7089a2d9e11 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -1126,7 +1126,6 @@ mod tests { let (lower, upper) = loops.yet_to_unroll[0] .get_const_bounds(function, &loops.cfg) - .expect("should find bounds") .expect("bounds are numeric const"); assert_eq!(lower, FieldElement::from(0u32));