From a48673ba4c7fe73d889f669fa54bce80818dbeef Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Fri, 23 May 2025 17:04:56 +0000 Subject: [PATCH 1/4] chore: add an assertion when parsing SSA that all functions are well formed --- .../noirc_evaluator/src/ssa/ir/function.rs | 22 +++++++++++++++++++ .../src/ssa/parser/into_ssa.rs | 4 ++++ 2 files changed, 26 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index ac5a64bb2fd..7f6fe1749b0 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -231,6 +231,28 @@ impl Function { } }) } + + /// Asserts that the [`Function`] is well formed. + /// + /// Panics on malformed functions. + pub(crate) fn assert_valid(&self) { + let reachable_blocks = self.reachable_blocks(); + + // We assume that all functions have a single block which terminates with a `return` instruction. + let return_blocks: BTreeSet<_> = reachable_blocks + .iter() + .map(|block| { + // All blocks must has a terminator instruction of some sort. + let terminator = self.dfg[*block].terminator().unwrap_or_else(|| { + panic!("Function {} has no terminator in block {block}", self.id()) + }); + matches!(terminator, TerminatorInstruction::Return { .. }) + }) + .collect(); + if return_blocks.len() > 1 { + panic!("Function {} has multiple return blocks {return_blocks:?}", self.id()) + } + } } impl Clone for Function { diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 995f5e41386..7f434d7280b 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -503,6 +503,10 @@ impl Translator { // before each print. ssa.normalize_ids(); + for (_, function) in &ssa.functions { + function.assert_valid(); + } + ssa } From 7f7816ad587ce4e0e2132e330d23c0b9704067fc Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Fri, 23 May 2025 18:56:25 +0000 Subject: [PATCH 2/4] . --- compiler/noirc_evaluator/src/ssa/ir/function.rs | 2 +- compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 7f6fe1749b0..0d73326efdc 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -241,7 +241,7 @@ impl Function { // We assume that all functions have a single block which terminates with a `return` instruction. let return_blocks: BTreeSet<_> = reachable_blocks .iter() - .map(|block| { + .filter(|block| { // All blocks must has a terminator instruction of some sort. let terminator = self.dfg[*block].terminator().unwrap_or_else(|| { panic!("Function {} has no terminator in block {block}", self.id()) diff --git a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs index 7f434d7280b..f30e975a79f 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/into_ssa.rs @@ -503,7 +503,7 @@ impl Translator { // before each print. ssa.normalize_ids(); - for (_, function) in &ssa.functions { + for function in ssa.functions.values() { function.assert_valid(); } From 3230d2fb5e80fba6e49644b35d11c0ba99ec9210 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 27 May 2025 10:57:00 +0000 Subject: [PATCH 3/4] . --- compiler/noirc_evaluator/src/ssa/ir/function.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/function.rs b/compiler/noirc_evaluator/src/ssa/ir/function.rs index 0d73326efdc..feeb34bc036 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/function.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/function.rs @@ -242,8 +242,8 @@ impl Function { let return_blocks: BTreeSet<_> = reachable_blocks .iter() .filter(|block| { - // All blocks must has a terminator instruction of some sort. - let terminator = self.dfg[*block].terminator().unwrap_or_else(|| { + // All blocks must have a terminator instruction of some sort. + let terminator = self.dfg[**block].terminator().unwrap_or_else(|| { panic!("Function {} has no terminator in block {block}", self.id()) }); matches!(terminator, TerminatorInstruction::Return { .. }) From 23a40ba282e787fa7a2d2b6aff309d76813a3033 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 27 May 2025 11:41:16 +0000 Subject: [PATCH 4/4] chore: update tests to match expectations of SSA --- .../noirc_evaluator/src/ssa/opt/inlining.rs | 24 ++++++++++++++++--- .../src/ssa/opt/simplify_cfg.rs | 2 +- .../noirc_evaluator/src/ssa/parser/tests.rs | 8 ++++--- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs index a5e1147d288..1f8060f0c3b 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/inlining.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/inlining.rs @@ -816,12 +816,14 @@ mod test { v2 = lt v1, u32 1 jmpif v2 then: b1, else: b2 b1(): - return u32 1 + jmp b3(u32 1) b2(): v4 = sub v1, u32 1 v5 = call f1(v4) -> u32 v6 = mul v1, v5 - return v6 + jmp b3(v6) + b3(v7: u32): + return v7 } "; let ssa = Ssa::from_str(src).unwrap(); @@ -842,7 +844,23 @@ mod test { b5(): jmp b6() b6(): - return u32 120 + jmp b7(u32 1) + b7(v0: u32): + jmp b8(v0) + b8(v1: u32): + v8 = mul u32 2, v1 + jmp b9(v8) + b9(v2: u32): + v10 = mul u32 3, v2 + jmp b10(v10) + b10(v3: u32): + v12 = mul u32 4, v3 + jmp b11(v12) + b11(v4: u32): + v14 = mul u32 5, v4 + jmp b12(v14) + b12(v5: u32): + return v5 } "); } diff --git a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs index 8c6fff7c561..3dbb219d6fc 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/simplify_cfg.rs @@ -349,7 +349,7 @@ mod test { b1(): return Field 1 b2(): - return Field 2 + jmp b1() } "; let ssa = Ssa::from_str(src).unwrap(); diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index 8f23d0b1d47..13a6d8ff1e6 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -170,7 +170,7 @@ fn test_jmpif() { b0(v0: Field): jmpif v0 then: b1, else: b2 b1(): - return + jmp b2() b2(): return } @@ -182,7 +182,7 @@ fn test_jmpif() { b0(v0: Field): jmpif v0 then: b2, else: b1 b1(): - return + jmp b2() b2(): return } @@ -197,10 +197,12 @@ fn test_multiple_jmpif() { b0(v0: Field, v1: Field): jmpif v0 then: b1, else: b2 b1(): - return + jmp b4() b2(): jmpif v1 then: b3, else: b1 b3(): + jmp b4() + b4(): return } ";