feat(ssa): Mark transitively dead parameters during DIE

[vezenovm]

Description

Problem*

Resolves #8239 (comment)

Summary*

Transtively dead parameters will now be removed during DIE.
For example this code:

Details

        brillig(inline) predicate_pure fn main f0 {
          b0(v0: i16):
            v5 = lt i16 3, v0
            jmpif v5 then: b1, else: b2
          b1():
            v8 = lt i16 4, v0
            jmpif v8 then: b3, else: b4
          b2():
            jmp b5(Field 3)
          b3():
            jmp b6(Field 1)
          b4():
            jmp b6(Field 2)
          b5(v1: Field):
            v12 = lt i16 5, v0
            jmpif v12 then: b7, else: b8
          b6(v2: Field):
            jmp b5(v2)
          b7():
            jmp b9(v1)
          b8():
            v13 = add v1, Field 1
            jmp b9(v13)
          b9(v3: Field):
            return
        }

Will become the following:

Details

        brillig(inline) predicate_pure fn main f0 {
          b0(v0: i16):
            v2 = lt i16 3, v0
            jmpif v2 then: b1, else: b2
          b1():
            v4 = lt i16 4, v0
            jmpif v4 then: b3, else: b4
          b2():
            jmp b5()
          b3():
            jmp b6()
          b4():
            jmp b6()
          b5():
            v6 = lt i16 5, v0
            jmpif v6 then: b7, else: b8
          b6():
            jmp b5()
          b7():
            jmp b9()
          b8():
            jmp b9()
          b9():
            return
        }

This was done in the following way:

1. A keep list was maintained per block during DIE.
   As per the doc comments:

    /// A per-block list indicating which block parameters are still considered alive.
    ///
    /// Each entry maps a [BasicBlockId] to a `Vec<bool>`, where the `i`th boolean corresponds to
    /// the `i`th parameter of that block. A value of `true` means the parameter is used and should
    /// be preserved. A value of `false` means it is unused and can be pruned.
    ///
    /// This keep list is used during terminator analysis to avoid incorrectly marking values as used
    /// simply because they appear as terminator arguments. Only parameters marked as live here
    /// should result in values being marked as used in terminator arguments.

2. DIE now runs in a feedback loop with pruning. This was a conscious choice to keep DIE and pruning separated and allow for easier testing of the separate passes.
3. Preprocess functions had to be updated to not run DIE in isolation on a function as we now mark terminator arguments potentially unused during DIE. It must be run over the full SSA.

Additional Context

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[github-actions]

ACVM Benchmarks

Details

Benchmark suite	Current: 8c5e7e9	Previous: a593e48	Ratio
purely_sequential_opcodes	273376 ns/iter (± 476)	264740 ns/iter (± 520)	1.03
perfectly_parallel_opcodes	244789 ns/iter (± 4233)	234841 ns/iter (± 3881)	1.04
perfectly_parallel_batch_inversion_opcodes	3584084 ns/iter (± 13150)	3221519 ns/iter (± 4324)	1.11

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Compilation Time

Details

Benchmark suite	Current: 8c5e7e9	Previous: a593e48	Ratio
regression_4709	0.675 s	0.675 s	1
ram_blowup_regression	12.8 s	12.7 s	1.01
global_var_regression_entry_points	0.358 s	0.49 s	0.73
private-kernel-inner	2.322 s	2.212 s	1.05
private-kernel-reset	6.738 s	6.55 s	1.03
private-kernel-tail	1.126 s	1.166 s	0.97
rollup-base-private	17.92 s	18.92 s	0.95
rollup-base-public	13.1 s	13.32 s	0.98
rollup-block-root-empty	1.282 s	1.256 s	1.02
rollup-block-root-single-tx	124 s	124 s	1
rollup-block-root	119 s	135 s	0.88
rollup-merge	1.066 s	1.14 s	0.94
rollup-root	1.62 s	1.67 s	0.97

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Execution Time

Details

Benchmark suite	Current: 8c5e7e9	Previous: a593e48	Ratio
private-kernel-inner	0.028 s	0.027 s	1.04
private-kernel-reset	0.164 s	0.163 s	1.01
private-kernel-tail	0.016 s	0.016 s	1
rollup-base-private	0.331 s	0.333 s	0.99
rollup-base-public	0.212 s	0.212 s	1
rollup-block-root	11.4 s	11.6 s	0.98
rollup-merge	0.004 s	0.004 s	1
rollup-root	0.013 s	0.013 s	1

This comment was automatically generated by workflow using github-action-benchmark.

[github-actions]

Test Suite Duration

Details

Benchmark suite	Current: 8c5e7e9	Previous: a593e48	Ratio
test_report_AztecProtocol_aztec-packages_noir-projects_aztec-nr	54 s	52 s	1.04
test_report_AztecProtocol_aztec-packages_noir-projects_noir-contracts	90 s	98 s	0.92
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_blob	42 s	39 s	1.08
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_private-kernel-lib	178 s	172 s	1.03
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_rollup-lib	191 s	186 s	1.03
test_report_AztecProtocol_aztec-packages_noir-projects_noir-protocol-circuits_crates_types	54 s	55 s	0.98
test_report_noir-lang_noir-bignum_	412 s	401 s	1.03
test_report_noir-lang_noir_bigcurve_	230 s	222 s	1.04
test_report_noir-lang_sha512_	28 s	27 s	1.04
test_report_zkpassport_noir_rsa_	22 s	22 s	1

This comment was automatically generated by workflow using github-action-benchmark.

[vezenovm]

to_bytes_consistent_inliner_zero 122 (+53) +76.81%

I will make a follow-up issue for this regression. This actually looks to be due DIE being run over the full SSA during the preprocess fns pass which is changing inlining.

EDIT: #8321

[asterite]

LGTM

[vezenovm]

arb_program_can_be_executed looks to be failing with a panic post the last master merge. Going to need to investigate that failure before merging.

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: a2c22d6	Previous: ff795cf	Ratio
test_report_noir-lang_noir_bigcurve_	285 s	235 s	1.21

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[vezenovm]

arb_program_can_be_executed looks to be failing with a panic post the last master merge. Going to need to investigate that failure before merging.

Here is the panic we see triggered during that test. It can be triggered on master so I am going to resolve it separately. It is currently blocking this PR.

[vezenovm]

Here is the panic we see triggered during that test. It can be triggered on master so I am going to resolve it separately. It is currently blocking this PR.

No longer getting a panic on the 0xd683001500100000 seed after #8426 and arb_program_can_be_executed passes.