Inconsistencies on whether `Instruction::MakeArray` should be hoisted in brillig runtimes

[TomAFrench]

In can_be_hoisted we say that MakeArray instructions cannot be hoisted unless in an ACIR runtime.

noir/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

Lines 1132 to 1136 in 3629a25

	// Arrays can be mutated in unconstrained code so code that handles this case must
	// take care to track whether the array was possibly mutated or not before
	// hoisted. Since we don't know if the containing pass checks for this, we
	// can only assume these are safe to hoist in constrained code.
	MakeArray { .. } => function.runtime().is_acir(),

Despite this, we have handling for hoisting a MakeArray instruction in hoist_loop_invariants

noir/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

Lines 296 to 316 in 3629a25

	// If we are hoisting a MakeArray instruction,
	// we need to issue an extra inc_rc in case they are mutated afterward.
	if self.inserter.function.runtime().is_brillig()
	&& matches!(
	self.inserter.function.dfg[instruction_id],
	Instruction::MakeArray { .. }
	)
	{
	let result =
	self.inserter.function.dfg.instruction_results(instruction_id)[0];
	let inc_rc = Instruction::IncrementRc { value: result };
	let call_stack = self
	.inserter
	.function
	.dfg
	.get_instruction_call_stack_id(instruction_id);
	self.inserter
	.function
	.dfg
	.insert_instruction_and_results(inc_rc, *block, None, call_stack);
	}

This implies that hoisting these instruction was blocked at some point without cleaning up previous handling.

[michaeljklein]

The comments for this line:

noir/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

Line 1136 in 3629a25

MakeArray { .. } => function.runtime().is_acir(),

were updated here:
14659f3#diff-59dfbe76f4a51707718b73277feb62d27b3ee9114733a6c4cb4906baeffdd1a7R178-R180

        // hoisted. We know that in this module we will insert a corresponding `IncRc` to
        // allow safe hoisting in unconstrained code, but just in case we would expose this
        // function to other modules, we return a more restrictive result here.
        MakeArray { .. } => function.runtime().is_acir().into(),

as part of #9718

I tested removing this handling code (at commit f25ebd6101):

noir/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

Lines 431 to 451 in 14c8a0f

	// If we are hoisting a MakeArray instruction,
	// we need to issue an extra inc_rc in case they are mutated afterward.
	if self.inserter.function.runtime().is_brillig()
	&& matches!(
	self.inserter.function.dfg[instruction_id],
	Instruction::MakeArray { .. }
	)
	{
	let result =
	self.inserter.function.dfg.instruction_results(instruction_id)[0];
	let inc_rc = Instruction::IncrementRc { value: result };
	let call_stack = self
	.inserter
	.function
	.dfg
	.get_instruction_call_stack_id(instruction_id);
	self.inserter
	.function
	.dfg
	.insert_instruction_and_results(inc_rc, *block, None, call_stack);
	}

and some execution_success tests errored without it, e.g. test_array_dedup_regression, test_poseidon_bn254_hash_width_3, and test_regression_5252