chore(audit): Fix vector items offset and other refactors

[aakoshh]

Description

Problem*

There are quite a few competing naming conventions under BrilligBlock::convert_ssa_instruction: binary_gen, convert_ssa_binary, array_get, codegen_allocate, etc, and handling instructions inline. It makes it more difficult than it needs to be to recognise what we are looking at.

Also noticed one bug with vector items offset.

Summary*

• Make sure all code generators that convert_ssa_instruction dispatches to are named codegen_<instruction> and extract the inline ones into their own method with docs.
• Add docs around array and vector pointer codegen.
• Add offsets module so we don't have to hint as much at the layouts in comments at various places:
  • VectorAddress and ArrayAddress were moved into memory.rs, using the new offsets module
  • Fixed VectorAddress::items_start() to use 3 instead of 2. There were no integration tests failing, but eyeballing the code revealed no reason why they should be different. A unit test failed, but that one manually constructed data without capacity in the meta-data; it is updated now.
• Added an assertion that a foreign call does not use an array/vector following another vector output: if it did, they would both be allocated the same memory pointer.
• Added notes about how I think the way memory is allocated for nested arrays works. For a moment I thought I mistakenly deallocate array addresses, which then the next nested array field overwrites, but I don't think that's the case. The arrays allocated on the stack for nested fields only need to survive until we take the value written into them, pointing at the heap.

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.

[socket-security]

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

[github-actions]

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: f54623b	Previous: 6826de8	Ratio
perfectly_parallel_batch_inversion_opcodes	2794162 ns/iter (± 2138)	2259734 ns/iter (± 10347)	1.24

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

CC: @TomAFrench

[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: a8e3e6a	Previous: 9cc8190	Ratio
test_report_zkpassport_noir-ecdsa_	2 s	1 s	2

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

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: dd1561e	Previous: 52b341d	Ratio
rollup-tx-base-private	21.96 s	18.24 s	1.20

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

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

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

Benchmark suite	Current: 6dc6b93	Previous: 6826de8	Ratio
sha512-100-bytes	0.078 s	0.051 s	1.53

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

CC: @TomAFrench

[asterite]

Looks great!

[github-actions]

Changes to number of Brillig opcodes executed

Generated at commit: 8aa0d8cf9863dbd5eb4966f2456469652c0e15f3, compared to commit: 9cc81909aa6d1197198935de9f423ee8313ebcde

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
regression_9496_inliner_max	+1 ❌	+0.19%
regression_9496_inliner_min	+1 ❌	+0.19%
regression_9496_inliner_zero	+1 ❌	+0.19%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
regression_9496_inliner_max	515 (+1)	+0.19%
regression_9496_inliner_min	515 (+1)	+0.19%
regression_9496_inliner_zero	515 (+1)	+0.19%

[github-actions]

Changes to Brillig bytecode sizes

Generated at commit: 8aa0d8cf9863dbd5eb4966f2456469652c0e15f3, compared to commit: 9cc81909aa6d1197198935de9f423ee8313ebcde

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
regression_9496_inliner_max	+1 ❌	+0.19%
regression_9496_inliner_min	+1 ❌	+0.19%
regression_9496_inliner_zero	+1 ❌	+0.19%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
regression_9496_inliner_max	541 (+1)	+0.19%
regression_9496_inliner_min	541 (+1)	+0.19%
regression_9496_inliner_zero	541 (+1)	+0.19%

[aakoshh]

I tracked down that +1 opcode in #10294 (comment) to be caused by c89b2ea

It can be undone if we rearrange the code like this:

            Value::Intrinsic(Intrinsic::SlicePopFront) => {
                // ...elements, target_len, target_vector = slice_pop_front len, vector
                let target_len = get_target_len(self, element_size);
                let pop_variables = vecmap(&results[0..element_size], |result| {
                    self.variables.define_variable(
                        self.function_context,
                        self.brillig_context,
                        *result,
                        dfg,
                    )
                });
                // Doing this (but not target_len), after the pop_variables, saves 1 opcode in one of the tests.
                let target_vector = get_target_vector(self, element_size + 1);

It's a bit obscure, so I'd rather just keep it as it is.

Looking at the opcodes, I think it's down to luck: we allocate stack memory in different order, and in the very end we end up with 3 MOV before the RETURN versus 2 MOVs if we move target_vector down. I reckon it's down to how codegen_register_to_register_moves eliminates no-op moves, so in once scenario there probably was a move where the source and the destination was the same, and in the other there wasn't.