chore(ssa): Nits in remove_bit_shift and remove_if_else

[aakoshh]

Description

Problem*

Part of auditing.

Summary*

• Fixes some typos in remove_bit_shift and remove_if_else
• Some refactoring in remove_bit_shift:
  • Makes get_value_max_num_bits recursive, which has also been added to fix: correct max_bit_size when left-shifting #9770
  • Seek clarification on why the 1-complement and 2-complement logic seems to do the opposite of how the literature describes we should do conversions, and add it to the docs. This was introduced in 2247814 .
  • Got the source from @guipublic 🙏 https://dspace.mit.edu/bitstream/handle/1721.1/6090/AIM-378.pdf
  • Added an extended explanation, thanks for the hints Guillaume 🍻
• Some refactoring in remove_if_else:
  • Make all slice capacity calculations into remove_if_else, leaving the ValueMerger free of it.
  • Only store the capacity of slices, not arrays.
  • Unify the SizeChange to always return just IDs.
  • Move assertions into the pre-check

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]

⚠️ 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: 85d08db	Previous: 574eba8	Ratio
sha512-100-bytes	0.094 s	0.054 s	1.74

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: f5f411a	Previous: 03d9dac	Ratio
test_report_zkpassport_noir-ecdsa_	2 s	1 s	2

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

CC: @TomAFrench

[asterite]

We should run benchmarks here because the last time I tried making this recursive it considerably slowed things down. That's why there's memoization in the other place where this happens.

[aakoshh]

I think that’s because you handled Binary, whereas I just added support for the unlikely case of a chain of casts, potentially originating from a constant.

I did it more just because it seems like an obvious place where some sort of recursion is needed, but I don’t expect it to be common

[TomAFrench]

I just added support for the unlikely case of a chain of casts

I think that the dfg flattens these whenever we encounter chains for casts anyway so I'd expect this to not fire.

[aakoshh]

I think that the dfg flattens these whenever we encounter chains for casts anyway so I'd expect this to not fire.

Fair enough. I think another case recursion handles is casting a constant to another type, for example:

v1 = u8 7
v2 = cast v1 as u32

For this with recursion it will now return 3 instead of 8.

I can imagine that the simplification does some magic on this too, but I reckon it's easier to see that the recursive version would work for constants across casts (which are the only two cases it handles, other than types), without having to take into account all the other machinery.

Put differently, I'd expect an auditor to highlight the that this method is incomplete, when looked at in isolation, so I thought better handle something that might never come up, rather than have to explain why we think this is fine.

[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: ca15cbc	Previous: fabd2af	Ratio
sha512-100-bytes	2.463 s	1.872 s	1.32

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

CC: @TomAFrench