Inlining: Panics when inlining a function with multiple return blocks

[vezenovm]

Aim

We have a validation check that a function only has a single return block, however, we do have handling for multiple return blocks in the inlining pass. If we were ever wanted our SSA to support multiple returns blocks (LLVM and cranelift both support an arbitrary number of return instructions) this code becomes relevant again.

Expected Behavior

    #[test]
    fn multiple_return_blocks() {
        let src = "
        brillig(inline) fn main f0 {
          b0(v0: u1):
            v1 = call f1(v0) -> Field
            return v1
        }
        brillig(inline) fn foo f1 {
          b0(v0: u1):
            jmpif v0 then: b1, else: b2
          b1():
            return Field 1
          b2():
            return Field 2
        }
        ";
        let ssa = Ssa::from_str_no_validation(src).unwrap();
        let _ = ssa.inline_functions(i64::MAX).unwrap();
    }

The following program, should at minimum not panic and complete the inlining pass. Ideally f1 would be inlined into f0.

Bug

The snippet above gives the following panic:

thread 'ssa::opt::inlining::test::multiple_return_blocks' panicked at compiler/noirc_evaluator/src/ssa/opt/inlining.rs:641:9:
assertion `left == right` failed
  left: 1
 right: 0
stack backtrace:
   0: rust_begin_unwind
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/std/src/panicking.rs:692:5
   1: core::panicking::panic_fmt
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/core/src/panicking.rs:75:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/4d91de4e48198da2e33413efdcd9cd2cc0c46688/library/core/src/panicking.rs:364:5
   4: noirc_evaluator::ssa::opt::inlining::PerFunctionContext::insert_new_instruction_results
             at ./src/ssa/opt/inlining.rs:641:9
   5: noirc_evaluator::ssa::opt::inlining::PerFunctionContext::inline_function
             at ./src/ssa/opt/inlining.rs:604:9
   6: noirc_evaluator::ssa::opt::inlining::PerFunctionContext::inline_block_instructions
             at ./src/ssa/opt/inlining.rs:500:33
   7: noirc_evaluator::ssa::opt::inlining::PerFunctionContext::inline_blocks
             at ./src/ssa/opt/inlining.rs:441:13

The panic comes from this assertion

noir/compiler/noirc_evaluator/src/ssa/opt/inlining.rs

Line 641 in b61f5eb

assert_eq!(old_results.len(), new_results.len());

It looks like the panic is a result of returning the wrong values from handle_function_returns:

noir/compiler/noirc_evaluator/src/ssa/opt/inlining.rs

Line 478 in b61f5eb

self.context.builder.block_parameters(return_block).to_vec()

The block parameters of the return_block are going to be zero (we just inserted the block). It looks like we are attempting to account for that our SSA requires a single return point by building a join block to have a unified return.

A couple options:

• Explore a separate pass or utility function that modifies the CFG to have a single return point that is separate from inlining. This could be used by inlining, simplfiy cfg, etc. We can then also remove a restriction from our IR.
• Modify inlining to only expect a single return block. Building a unified return block can come later if that restriction is relaxed

To Reproduce

1. Copy the test above into the inlining test module
2. Run the test

Workaround

None

Workaround Description

No response

Additional Context

No response

Project Impact

None

Blocker Context

No response

Nargo Version

nargo version = 1.0.0-beta.9 noirc version = 1.0.0-beta.9+cceeec0d4f379632eae2f3228d2b8cefb07af7ee (git version hash: 8146755, is dirty: true)

NoirJS Version

No response

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response