fix: Use get_local_or_global_instruction when simplifying IfElse

[aakoshh]

Description

Problem*

Resolves #8174

Summary*

Changes the way IfElse is simplified so that the then_value and else_value are looked up with DataFlowGraph::get_local_or_global_instruction, directly returning an Instruction from either dfg or dfg.globals, rather than the two step process where we 1) get the Value::Instruction transparently from dfg or dfg.global and then 2) go to dfg to get the instruction.

Thanks @vezenovm for the tip about this method 🙏

Additional Context

See #8174 (comment) for an illustration of how the issue arises:

1. We see a ValueId in the terminator of a block in the SSA, which in this case is a global ID passing an array to the next block.
2. The Index by ValueId of DataFlowGraph checks if the ID is global. If it is, it's returned from the GlobalsGraph instead of DataFlowGraph::values.
3. The value in this case was a Value::Instruction, which carries an InstructionId that we need to look up, however the ID doesn't say if it's a global or a local instruction. Because it came from a global ValueId, it should have been looked up in dfg.globals, not dfg.

Any others?

I had a brief look at other instances, looking for other potential places this method could be used. Perhaps simplify::try_optimize_array_set_from_previous_get? It doesn't seem to have the short circuit of global arrays considered immutable like constant_folding::remove_possibly_mutated_cached_make_arrays does.

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.

[vezenovm]

Perhaps simplify::try_optimize_array_set_from_previous_get? It doesn't seem to have the short circuit of global arrays considered immutable like constant_folding::remove_possibly_mutated_cached_make_arrays does.

We shouldn't have any array set instructions acting on global arrays due to their immutable nature.

[vezenovm]

Looks good!

[aakoshh]

We shouldn't have any array set instructions acting on global arrays due to their immutable nature.

Yes, what I wasn't sure about is that in constant_folding we check that the instruction is an ArraySet and then skip if it's global (which shouldn't even happen, but it checks it nevertheless), while in try_optimize_array_set_from_previous_get this possibility is not entertained, unlike in try_optimize_array_get_from_previous_set where it is. As long as you're happy with that, so am I 🫡

[vezenovm]

constant_folding we check that the instruction is an ArraySet and then skip if it's global (which shouldn't even happen, but it checks it nevertheless)

We also are checking whether the instruction is a Store.

while in try_optimize_array_set_from_previous_get this possibility is not entertained, unlike in try_optimize_array_get_from_previous_set where it is

I have thought about switching all instances of dfg[instruction_id] with get_local_or_global_instruction to prevent the possibility of these kinds of panics. I ultimately went with just using get_local_or_global_instruction wherever we may have a global MakeArray instruction as to avoid too many pervasive changes. I wouldn't be against experimenting with making this switch but it should be a separate PR.