fix(ssa): don't deduplicate constraints in blocks that are not dominated

[asterite]

Description

Problem

Resolves #6588

Summary

The PR fixes two issues in constant_folding introduced by #6499 :

1. The constraint_simplification_mapping didn't take into account the block in which the constraint was encountered to limit where the simplification can be used.
2. Hoisting instructions into the common dominator should only happen when the instruction is free of side effects, because there can be other paths in which it's invalid.

Simplification

Here's an example where things went wrong:

fn main(x: Field, y: bool) {
    if y {
        assert_eq(x, 1);
    }
    print(x);
}

Calling the above with main(0, false) prints 1 on the console, because even though the body of the if is not hit, the constrain x, 1 ended up taking effect outside the if and "simplified" print(x) to print(1).

The PR fixes this by remembering that the constraint was encountered in the block corresponding to the then branch, and only uses the simplification in blocks which are dominated by it.

Hoisting

For this to kick in we needed two blocks have the same expression:

fn main(x: Field, y: bool, z: bool) {
    if y {
        assert_eq(x, 1); // Caches the result of the instruction
    } 
    if z {
        assert_eq(x, 1); // Finds the result in the cache, but it's from a sibling block, so it hoists it up in the common dominator
    }
}

Calling the above with main(0, false, false) causes the circuit to fail, because it is rewritten to something like this:

fn main(x: Field, y: bool, z: bool) {
    assert_eq(x, 1); // Instruction hoisted to the block before both `if`s
    if y {
        assert_eq(x, 1); // Original instruction still in place until the next deduplication
    } 
    if z {
        // Instruction result now reusable from cache
    }
    // Now we enforce the constraint even when both y and z are false
}

The PR fixes this by only hoisting instructions which don't have side effects (such as failing the circuit, or modifying an array).

Additional Context

Curiously the results cache does take into account the side effect variable a.k.a. predicate:

type InstructionResultCache = HashMap<Instruction, HashMap<Option<ValueId>, ResultCache>>;
                                                             ^
                                                             optional predicate

So it should be able to isolate the results based on whether we are before the if, in the then or in the else block. However it's use depends on whether Instruction::requires_acir_gen_predicate indicates that it should be used, and for Constrain this is not the case, so it always ends up being stored with None for key.

            let use_predicate =
                self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg);
            let predicate = use_predicate.then_some(side_effects_enabled_var);

            self.cached_instruction_results
                .entry(instruction)
                .or_default()
                .entry(predicate)
                .or_default()
                .cache(block, instruction_results);

NB previously every block had its own cache, so maybe the intent behind this was completely different, but it would be good to have clarification on how flatten_cfg, enable_side_effect, and this, are supposed to work together.

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.

[TomAFrench]

Nice! Good find.

[github-actions]

Changes to Brillig bytecode sizes

Generated at commit: b4480765b1f8b4e7c771b1a1e1ff6eaefcbd6622, compared to commit: dfc9ff7266d2b6694cae3da88418013664440daa

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
fold_call_witness_condition	+4 ❌	+3.64%
slice_dynamic_index	+12 ❌	+0.48%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
fold_call_witness_condition	114 (+4)	+3.64%
slice_dynamic_index	2,532 (+12)	+0.48%

[github-actions]

Changes to number of Brillig opcodes executed

Generated at commit: b4480765b1f8b4e7c771b1a1e1ff6eaefcbd6622, compared to commit: dfc9ff7266d2b6694cae3da88418013664440daa

🧾 Summary (10% most significant diffs)

Program	Brillig opcodes (+/-)	%
fold_call_witness_condition	+1 ❌	+1.45%
slice_regex	-44 ✅	-1.28%

---

Full diff report 👇

Program	Brillig opcodes (+/-)	%
fold_call_witness_condition	70 (+1)	+1.45%
uhashmap	146,340 (-3)	-0.00%
hashmap	52,936 (-3)	-0.01%
slice_dynamic_index	4,305 (-26)	-0.60%
slice_regex	3,390 (-44)	-1.28%

[TomAFrench]

@asterite @aakoshh Can you pull these fixes into AztecProtocol/aztec-packages#9972?

[asterite]

I already pulled some early fixes into it. That problem gets solved but it fails with another one that gets fixed with #6585 . Unfortunately that PR introduces huge regressions so I don't know what's the path forward.

That said, we could try merging the changes here into AztecProtocol/aztec-packages#9972 anyway.

[asterite]

Done! I couldn't apply all the changes here because the hoisting logic doesn't exist yet in Aztec-Packages.