fix(acir-gen): Use current side effect predicate in memory ops for slices

[aakoshh]

Description

Problem*

Resolves #9467

Summary*

Changes the SSA-to-ACIR codegen handling of Intrinsics::SlicePopFront and other slice operations to pass current_side_effects_var when creating the ACIR Opcode::MemoryOp, so that if side effects are disabled, then nothing happens. This way trying to pop from an empty slice on a disabled branch doesn't cause circuit failure.

Failing circuit size calculations in BB

There is some strange failure somewhere in bb.

Additional Context

I'm not sure why the same predicate wasn't used before for array operations. I only enabled it for slices, because for arrays the predicate seems to become part of the indexing itself.

Another approach would be to insert constraints into SSA like in #9323 but in that PR we discussed that these should only be required for Brillig, so I thought ACIR should be able to handle this without extra constraints.

Yet another alternative would be for remove_unreachable_instructions to look for 0 size, but I figured that would i) fragment the knowledge about these intrinsics even further, ii) it wouldn't be much different from inserting the constraints up front and iii) wouldn't cover popping from slices where the compiler doesn't statically know that the size is 0.

With these changes the ACIR for the program in the bug ticket looks like this:

Compiled ACIR for main (unoptimized):
func 0
current witness index : _3
private parameters indices : [_0]
public parameters indices : []
return value indices : [_1]
BLACKBOX::RANGE [(_0, 1)] []
INIT (id: 0, len: 0, witnesses: [])
EXPR [ (-1, _2) 0 ]
MEM PREDICATE: EXPR [ (1, _0) 0 ]
(id: 0, read at: EXPR [ (1, _2) 0 ], value: EXPR [ (1, _3) 0 ]) 
EXPR [ (-1, _0, _3) (1, _0) (1, _1) -1 ]

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: 86c73a7	Previous: e54057d	Ratio
private-kernel-inner	0.016 s	0.013 s	1.23

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

CC: @TomAFrench

[TomAFrench]

The predicate on memory ops is currently ignored by barretenberg and is being removed in #8134

[aakoshh]

@TomAFrench thanks for the heads up. Should I insert constraints into the SSA the way we do for Brillig then?

[TomAFrench]

Yeah, I think we're going to need to explicitly guard in cases where the slice length can potentially go to zero.

[aakoshh]

Okay, I'll close this and open an alternative PR.

A quick test with adding the same intrinsic call check as we did in Brillig revealed that it won't work on its own for ACIR. It resuls in an SSA like this:

    enable_side_effects v0
    constrain u1 0 == v0, "Index out of bounds"   	// src/main.nr:4:9
    v7, v8, v9 = call slice_pop_front(u32 0, v1) -> (u32, u32, [u32])	// src/main.nr:4:9

So it requires that we don't go into the if, because that would result in IOOB, but then the slice_pop_front fails anyway because it doesn't care about the side effect variable (which is also removed, because the constraint doesn't care about it either).

I'll see if we can somehow reason about being under a predicate in remove_unreachable_instructions to replace the values with defaults, but I'm not sure how this can be handled without the ACVM being told not to pop.

[aakoshh]

Closing in favour of #9489