From abbd88d65f791686d831af99d2731db7e1600a96 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 5 Dec 2024 16:27:39 -0300 Subject: [PATCH 1/7] fix: print ssa blocks without recursion --- .../noirc_evaluator/src/ssa/ir/printer.rs | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index aa2952d5abc..364d4a4f3a7 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -1,6 +1,6 @@ //! This file is for pretty-printing the SSA IR in a human-readable form for debugging. use std::{ - collections::HashSet, + collections::{HashSet, VecDeque}, fmt::{Formatter, Result}, }; @@ -34,14 +34,22 @@ pub(crate) fn display_block_with_successors( visited: &mut HashSet, f: &mut Formatter, ) -> Result { - display_block(function, block_id, f)?; - visited.insert(block_id); + // The block chain to print might be really long so we use a deque instead of recursion + // to avoid potentially hitting stack overflow (see https://github.com/noir-lang/noir/issues/6520) + let mut blocks_to_print = VecDeque::new(); + blocks_to_print.push_back(block_id); - for successor in function.dfg[block_id].successors() { - if !visited.contains(&successor) { - display_block_with_successors(function, successor, visited, f)?; + while let Some(block_id) = blocks_to_print.pop_front() { + if !visited.insert(block_id) { + continue; + }; + display_block(function, block_id, f)?; + + for successor in function.dfg[block_id].successors() { + blocks_to_print.push_back(successor); } } + Ok(()) } From 65908370e242298f6e3708642784cd846eae6d5b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 5 Dec 2024 16:32:57 -0300 Subject: [PATCH 2/7] Simpler function --- .../noirc_evaluator/src/ssa/ir/printer.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 364d4a4f3a7..eb7783177fc 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -21,23 +21,20 @@ use super::{ /// Helper function for Function's Display impl to pretty-print the function with the given formatter. pub(crate) fn display_function(function: &Function, f: &mut Formatter) -> Result { writeln!(f, "{} fn {} {} {{", function.runtime(), function.name(), function.id())?; - display_block_with_successors(function, function.entry_block(), &mut HashSet::new(), f)?; + display_function_blocks(function, f)?; write!(f, "}}") } -/// Displays a block followed by all of its successors recursively. -/// This uses a HashSet to keep track of the visited blocks. Otherwise -/// there would be infinite recursion for any loops in the IR. -pub(crate) fn display_block_with_successors( - function: &Function, - block_id: BasicBlockId, - visited: &mut HashSet, - f: &mut Formatter, -) -> Result { +/// Displays all of a function's blocks by printing the entry block +/// and its successors, recursively. +pub(crate) fn display_function_blocks(function: &Function, f: &mut Formatter) -> Result { // The block chain to print might be really long so we use a deque instead of recursion // to avoid potentially hitting stack overflow (see https://github.com/noir-lang/noir/issues/6520) let mut blocks_to_print = VecDeque::new(); - blocks_to_print.push_back(block_id); + blocks_to_print.push_back(function.entry_block()); + + // Keep track of the visited blocks. Otherwise there would be infinite recursion for any loops in the IR. + let mut visited = HashSet::new(); while let Some(block_id) = blocks_to_print.pop_front() { if !visited.insert(block_id) { From 431bacd6a666dc7e94c4cecc9632ef822561f9fd Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 5 Dec 2024 16:39:32 -0300 Subject: [PATCH 3/7] Less lines --- compiler/noirc_evaluator/src/ssa/ir/printer.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index eb7783177fc..a2d016c6bd6 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -25,8 +25,7 @@ pub(crate) fn display_function(function: &Function, f: &mut Formatter) -> Result write!(f, "}}") } -/// Displays all of a function's blocks by printing the entry block -/// and its successors, recursively. +/// Displays all of a function's blocks by printing the entry block and its successors, recursively. pub(crate) fn display_function_blocks(function: &Function, f: &mut Formatter) -> Result { // The block chain to print might be really long so we use a deque instead of recursion // to avoid potentially hitting stack overflow (see https://github.com/noir-lang/noir/issues/6520) From ee4961b04010f55e324283685fbf66e426be4431 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 6 Dec 2024 07:19:48 -0300 Subject: [PATCH 4/7] Use `reachable_blocks` --- .../noirc_evaluator/src/ssa/ir/printer.rs | 31 ++----------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index a2d016c6bd6..29e79728303 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -1,8 +1,5 @@ //! This file is for pretty-printing the SSA IR in a human-readable form for debugging. -use std::{ - collections::{HashSet, VecDeque}, - fmt::{Formatter, Result}, -}; +use std::fmt::{Formatter, Result}; use acvm::acir::AcirField; use im::Vector; @@ -21,32 +18,10 @@ use super::{ /// Helper function for Function's Display impl to pretty-print the function with the given formatter. pub(crate) fn display_function(function: &Function, f: &mut Formatter) -> Result { writeln!(f, "{} fn {} {} {{", function.runtime(), function.name(), function.id())?; - display_function_blocks(function, f)?; - write!(f, "}}") -} - -/// Displays all of a function's blocks by printing the entry block and its successors, recursively. -pub(crate) fn display_function_blocks(function: &Function, f: &mut Formatter) -> Result { - // The block chain to print might be really long so we use a deque instead of recursion - // to avoid potentially hitting stack overflow (see https://github.com/noir-lang/noir/issues/6520) - let mut blocks_to_print = VecDeque::new(); - blocks_to_print.push_back(function.entry_block()); - - // Keep track of the visited blocks. Otherwise there would be infinite recursion for any loops in the IR. - let mut visited = HashSet::new(); - - while let Some(block_id) = blocks_to_print.pop_front() { - if !visited.insert(block_id) { - continue; - }; + for block_id in function.reachable_blocks() { display_block(function, block_id, f)?; - - for successor in function.dfg[block_id].successors() { - blocks_to_print.push_back(successor); - } } - - Ok(()) + write!(f, "}}") } /// Display a single block. This will not display the block's successors. From a3ef6210ed0b240112ce364096151be88e68f963 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 6 Dec 2024 10:28:32 -0300 Subject: [PATCH 5/7] Let `unreachable_blocks` return the blocks in order of appearance --- .../noirc_evaluator/src/ssa/ir/function.rs | 9 ++- .../noirc_evaluator/src/ssa/opt/array_set.rs | 4 +- .../src/ssa/opt/constant_folding.rs | 12 +-- .../src/ssa/opt/loop_invariant.rs | 78 +++++++++---------- .../noirc_evaluator/src/ssa/opt/mem2reg.rs | 28 +++---- .../src/ssa/opt/simplify_cfg.rs | 6 +- .../noirc_evaluator/src/ssa/opt/unrolling.rs | 18 ++--- 7 files changed, 78 insertions(+), 77 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 6413107c04a..03b65b1075c 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, VecDeque}; use iter_extended::vecmap; use noirc_frontend::monomorphization::ast::InlineType; @@ -169,11 +169,12 @@ impl Function { /// want to iterate only reachable blocks. pub(crate) fn reachable_blocks(&self) -> BTreeSet { let mut blocks = BTreeSet::new(); - let mut stack = vec![self.entry_block]; + let mut deque = VecDeque::new(); + deque.push_back(self.entry_block); - while let Some(block) = stack.pop() { + while let Some(block) = deque.pop_front() { if blocks.insert(block) { - stack.extend(self.dfg[block].successors()); + deque.extend(self.dfg[block].successors()); } } blocks diff --git a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs index 96de22600a4..09339cf0797 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/array_set.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/array_set.rs @@ -209,6 +209,8 @@ mod tests { b1(v0: u32): v8 = lt v0, u32 5 jmpif v8 then: b3, else: b2 + b2(): + return b3(): v9 = eq v0, u32 5 jmpif v9 then: b4, else: b5 @@ -224,8 +226,6 @@ mod tests { store v15 at v4 v17 = add v0, u32 1 jmp b1(v17) - b2(): - return } "; let ssa = Ssa::from_str(src).unwrap(); diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 93ca428c6d0..e039b8f0f9e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -1521,18 +1521,18 @@ mod test { b0(v0: u32): v2 = eq v0, u32 0 jmpif v2 then: b4, else: b1 - b4(): - v5 = sub v0, u32 1 - jmp b5() - b5(): - return b1(): jmpif v0 then: b3, else: b2 + b2(): + jmp b5() b3(): v4 = sub v0, u32 1 // We can't hoist this because v0 is zero here and it will lead to an underflow jmp b5() - b2(): + b4(): + v5 = sub v0, u32 1 jmp b5() + b5(): + return } "; let ssa = Ssa::from_str(src).unwrap(); diff --git a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index 290d2a33846..87e7f8bcff3 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -251,13 +251,13 @@ mod test { b1(v2: u32): v5 = lt v2, u32 4 jmpif v5 then: b3, else: b2 + b2(): + return b3(): v6 = mul v0, v1 constrain v6 == u32 6 v8 = add v2, u32 1 jmp b1(v8) - b2(): - return } "; @@ -276,12 +276,12 @@ mod test { b1(v2: u32): v6 = lt v2, u32 4 jmpif v6 then: b3, else: b2 + b2(): + return b3(): constrain v3 == u32 6 v9 = add v2, u32 1 jmp b1(v9) - b2(): - return } "; @@ -300,21 +300,21 @@ mod test { b1(v2: u32): v6 = lt v2, u32 4 jmpif v6 then: b3, else: b2 + b2(): + return b3(): jmp b4(u32 0) b4(v3: u32): v7 = lt v3, u32 4 jmpif v7 then: b6, else: b5 + b5(): + v9 = add v2, u32 1 + jmp b1(v9) b6(): v10 = mul v0, v1 constrain v10 == u32 6 v12 = add v3, u32 1 jmp b4(v12) - b5(): - v9 = add v2, u32 1 - jmp b1(v9) - b2(): - return } "; @@ -333,20 +333,20 @@ mod test { b1(v2: u32): v7 = lt v2, u32 4 jmpif v7 then: b3, else: b2 + b2(): + return b3(): jmp b4(u32 0) b4(v3: u32): v8 = lt v3, u32 4 jmpif v8 then: b6, else: b5 + b5(): + v10 = add v2, u32 1 + jmp b1(v10) b6(): constrain v4 == u32 6 v12 = add v3, u32 1 jmp b4(v12) - b5(): - v10 = add v2, u32 1 - jmp b1(v10) - b2(): - return } "; @@ -374,6 +374,8 @@ mod test { b1(v2: u32): v5 = lt v2, u32 4 jmpif v5 then: b3, else: b2 + b2(): + return b3(): v6 = mul v0, v1 v7 = mul v6, v0 @@ -381,8 +383,6 @@ mod test { constrain v7 == u32 12 v9 = add v2, u32 1 jmp b1(v9) - b2(): - return } "; @@ -402,12 +402,12 @@ mod test { b1(v2: u32): v9 = lt v2, u32 4 jmpif v9 then: b3, else: b2 + b2(): + return b3(): constrain v4 == u32 12 v11 = add v2, u32 1 jmp b1(v11) - b2(): - return } "; @@ -431,17 +431,17 @@ mod test { b1(v2: u32): v7 = lt v2, u32 4 jmpif v7 then: b3, else: b2 + b2(): + v8 = load v5 -> [u32; 5] + v10 = array_get v8, index u32 2 -> u32 + constrain v10 == u32 3 + return b3(): v12 = load v5 -> [u32; 5] v13 = array_set v12, index v0, value v1 store v13 at v5 v15 = add v2, u32 1 jmp b1(v15) - b2(): - v8 = load v5 -> [u32; 5] - v10 = array_get v8, index u32 2 -> u32 - constrain v10 == u32 3 - return } "; @@ -485,16 +485,24 @@ mod test { b1(v2: u32): v9 = lt v2, u32 4 jmpif v9 then: b3, else: b2 + b2(): + return b3(): jmp b4(u32 0) b4(v3: u32): v10 = lt v3, u32 4 jmpif v10 then: b6, else: b5 + b5(): + v12 = add v2, u32 1 + jmp b1(v12) b6(): jmp b7(u32 0) b7(v4: u32): v13 = lt v4, u32 4 jmpif v13 then: b9, else: b8 + b8(): + v14 = add v3, u32 1 + jmp b4(v14) b9(): v15 = array_get v6, index v2 -> u32 v16 = eq v15, v0 @@ -504,14 +512,6 @@ mod test { constrain v17 == v0 v19 = add v4, u32 1 jmp b7(v19) - b8(): - v14 = add v3, u32 1 - jmp b4(v14) - b5(): - v12 = add v2, u32 1 - jmp b1(v12) - b2(): - return } "; @@ -526,6 +526,8 @@ mod test { b1(v2: u32): v9 = lt v2, u32 4 jmpif v9 then: b3, else: b2 + b2(): + return b3(): v10 = array_get v6, index v2 -> u32 v11 = eq v10, v0 @@ -533,6 +535,9 @@ mod test { b4(v3: u32): v12 = lt v3, u32 4 jmpif v12 then: b6, else: b5 + b5(): + v14 = add v2, u32 1 + jmp b1(v14) b6(): v15 = array_get v6, index v3 -> u32 v16 = eq v15, v0 @@ -540,19 +545,14 @@ mod test { b7(v4: u32): v17 = lt v4, u32 4 jmpif v17 then: b9, else: b8 + b8(): + v18 = add v3, u32 1 + jmp b4(v18) b9(): constrain v10 == v0 constrain v15 == v0 v19 = add v4, u32 1 jmp b7(v19) - b8(): - v18 = add v3, u32 1 - jmp b4(v18) - b5(): - v14 = add v2, u32 1 - jmp b1(v14) - b2(): - return } "; diff --git a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs index 53a31ae57c1..77ad53df9cf 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs @@ -1121,11 +1121,6 @@ mod tests { b1(v0: Field): v4 = eq v0, Field 0 jmpif v4 then: b3, else: b2 - b3(): - v11 = load v3 -> &mut Field - store Field 2 at v11 - v13 = add v0, Field 1 - jmp b1(v13) b2(): v5 = load v1 -> Field v7 = eq v5, Field 2 @@ -1135,6 +1130,11 @@ mod tests { v10 = eq v9, Field 2 constrain v9 == Field 2 return + b3(): + v11 = load v3 -> &mut Field + store Field 2 at v11 + v13 = add v0, Field 1 + jmp b1(v13) } "; @@ -1157,11 +1157,6 @@ mod tests { b1(v0: Field): v4 = eq v0, Field 0 jmpif v4 then: b3, else: b2 - b3(): - v13 = load v3 -> &mut Field - store Field 2 at v13 - v15 = add v0, Field 1 - jmp b1(v15) b2(): v5 = load v1 -> Field v7 = eq v5, Field 2 @@ -1173,6 +1168,11 @@ mod tests { v12 = eq v11, Field 2 constrain v11 == Field 2 return + b3(): + v13 = load v3 -> &mut Field + store Field 2 at v13 + v15 = add v0, Field 1 + jmp b1(v15) } acir(inline) fn foo f1 { b0(v0: &mut Field): @@ -1195,6 +1195,10 @@ mod tests { acir(inline) fn main f0 { b0(v0: u1): jmpif v0 then: b2, else: b1 + b1(): + v4 = allocate -> &mut Field + store Field 1 at v4 + jmp b3(v4, v4, v4) b2(): v6 = allocate -> &mut Field store Field 0 at v6 @@ -1212,10 +1216,6 @@ mod tests { constrain v11 == Field 1 constrain v13 == Field 3 return - b1(): - v4 = allocate -> &mut Field - store Field 1 at v4 - jmp b3(v4, v4, v4) } "; diff --git a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs index c282e2df451..e7f8d227d28 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs @@ -442,14 +442,14 @@ mod test { store Field 0 at v1 v3 = not v0 jmpif v0 then: b2, else: b1 + b1(): + store Field 2 at v1 + jmp b2() b2(): v5 = load v1 -> Field v6 = eq v5, Field 2 constrain v5 == Field 2 return - b1(): - store Field 2 at v1 - jmp b2() }"; assert_normalized_ssa_equals(ssa.simplify_cfg(), expected); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs index 1a13acc5435..22daba1de45 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/unrolling.rs @@ -1337,12 +1337,15 @@ mod tests { b2(): v7 = eq v0, u32 2 jmpif v7 then: b7, else: b3 - b7(): - v18 = add v0, u32 1 - jmp b1(v18) b3(): v9 = eq v0, u32 5 jmpif v9 then: b5, else: b4 + b4(): + v10 = load v1 -> Field + v12 = add v10, Field 1 + store v12 at v1 + v14 = add v0, u32 1 + jmp b1(v14) b5(): jmp b6() b6(): @@ -1350,12 +1353,9 @@ mod tests { v17 = eq v15, Field 4 constrain v15 == Field 4 return - b4(): - v10 = load v1 -> Field - v12 = add v10, Field 1 - store v12 at v1 - v14 = add v0, u32 1 - jmp b1(v14) + b7(): + v18 = add v0, u32 1 + jmp b1(v18) } "; let ssa = Ssa::from_str(src).unwrap(); From 936fd7b1663ad983e740064b72f7532bedd69099 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 6 Dec 2024 10:36:10 -0300 Subject: [PATCH 6/7] Revert reachable_blocks code to how it was --- compiler/noirc_evaluator/src/ssa/ir/function.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 03b65b1075c..6413107c04a 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, VecDeque}; +use std::collections::BTreeSet; use iter_extended::vecmap; use noirc_frontend::monomorphization::ast::InlineType; @@ -169,12 +169,11 @@ impl Function { /// want to iterate only reachable blocks. pub(crate) fn reachable_blocks(&self) -> BTreeSet { let mut blocks = BTreeSet::new(); - let mut deque = VecDeque::new(); - deque.push_back(self.entry_block); + let mut stack = vec![self.entry_block]; - while let Some(block) = deque.pop_front() { + while let Some(block) = stack.pop() { if blocks.insert(block) { - deque.extend(self.dfg[block].successors()); + stack.extend(self.dfg[block].successors()); } } blocks From 999d0719e684d007107fc8ac69cb56192c9a1982 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 6 Dec 2024 10:50:32 -0300 Subject: [PATCH 7/7] Fix another test --- compiler/noirc_evaluator/src/ssa/parser/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 6318f9dc56e..dab96dfa04f 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -143,10 +143,10 @@ fn test_jmpif() { acir(inline) fn main f0 { b0(v0: Field): jmpif v0 then: b2, else: b1 - b2(): - return b1(): return + b2(): + return } "; assert_ssa_roundtrip(src);