fix(ssa refactor): resolve replaced value ids for printing

[joss-aztec]

Description

Problem*

The SSA printer gave confusing output when a value had been replaced (during mem2reg).

Resolves #1451

Summary*

Track ids involved in value replacements such that the printer can resolve them later.

For program:

fn main(x: Field) -> pub Field {
    let xs = [x];
    xs[0]
}

Before:

fn main f1 {
  b0(v0: Field):
    v1 = alloc 1 fields
    return v2
}

After:

fn main f1 {
  b0(v0: Field):
    v1 = alloc 1 fields
    return v0
}

Documentation

• This PR requires documentation updates when merged.

  • I will submit a noir-lang/docs PR.
  • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

[jfecher]

I have to think about this some more. I think our handling of set_value_from_id may be incorrect in a deeper way. For example, each of the passes that map one ValueId to another would be incorrect if they thought they mapped each ValueId corresponding to an instruction's results but there were actually additional ids aliased to those results.

I'm thinking we may need to store the replaced ValueId in each value directly, but that would be somewhat unfortunate. Alternatively we could say instructions/values must be re-inserted rather than replaced via set_value but perhaps this is overkill.

[joss-aztec]

I've just realised this issue is breaking acir_gen too. (You can reproduce with 1_mul example.)

Initial SSA:
fn main f0 {
  b0(v0: u32, v2: u32, v3: u32):
    v1 = alloc 1 fields
    store v0 at v1
    v5 = load v1
    v6 = mul v5, v2
    
...

After Mem2Reg:
fn main f1 {
  b0(v0: u32, v1: u32, v2: u32):
    v3 = alloc 1 fields
    v5 = mul v4, v1

When it fetches v4 (i.e. should instead be v0) from ssa_value_to_acir_var, it fails because v4 wasn't bound with any AcirVar during convert_ssa_block_params.

This is going to break any program that uses a mutable parameter.

[jfecher]

It may be the case we should remove set_value and set_value_from_id entirely. Instead we'd have to manually find each instruction that references the value and mutate that instruction.

Alternatively, if we want to keep set_value, we'd have to avoid HashMap<ValueId, _> entirely and switch to e.g. HashMap<Value, _> directly. Or switch ValueId to be the equivalent of a Value** rather than a Value*.

[joss-aztec]

I'm more incline to remove set_value and set_value_from_id. It would be awkward when substitution is required, but I'd rather have to deal with that awkwardness in a contained manner, than have to introduce the extra mental overhead of thinking in terms of pointers to pointers everywhere else.

[joss-aztec]

Added issue for tracking: #1592

[kevaundray]

Revisiting this -- since we want the examples working end to end as a priority, if this fixes the issue, we should go with this for now as it's a fairly small change and prioritize a proper fix once examples are working

Edit: Seems it does not fix the issue

[jfecher]

Merging this PR should fix the issue with set_value, but getting the acir_gen tests working also requires PR #1595 to remove the remaining allocate instructions. That PR will need the fix provided from this PR as well.