diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 68c8cb18ee3..8619e588023 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -793,6 +793,17 @@ impl DataFlowGraph { _ => ArrayOffset::None, } } + + /// Check if the results of an instruction are used in the databus to return a value.. + /// + /// This only applies to ACIR, as in Brillig the databus will always be empty. + pub(crate) fn is_returned_in_databus(&self, instruction_id: InstructionId) -> bool { + let Some(return_data) = self.data_bus.return_data else { + return false; + }; + let results = self.instruction_results(instruction_id); + results.iter().any(|id| *id == return_data) + } } impl std::ops::Index for DataFlowGraph { diff --git a/compiler/noirc_evaluator/src/ssa/opt/die.rs b/compiler/noirc_evaluator/src/ssa/opt/die.rs index 9a0f9e5ecab..d8106d23ec6 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/die.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/die.rs @@ -25,10 +25,12 @@ //! multiple unused array accesses. As to avoid redundant OOB checks, we search for "array get groups" //! and only insert a single OOB check for an array get group. //! - [Store][Instruction::Store] instructions can only be removed if the `flattened` flag is set. +//! - Instructions that create the value which is returned in the databus (if present) is not removed. //! - Brillig //! - Array operations are explicit and thus it is expected separate OOB checks //! have been laid down. Thus, no extra instructions are inserted for unused array accesses. //! - [Store][Instruction::Store] instructions are never removed. +//! - The databus is never used to return values, so instructions to create a Field array to return are never generated. //! //! ## Preconditions //! - ACIR: By default the pass must be run after [mem2reg][crate::ssa::opt::mem2reg] and [CFG flattening][crate::ssa::opt::flatten_cfg]. @@ -344,7 +346,8 @@ impl Context { if can_be_eliminated_if_unused(instruction, function, self.flattened) { let results = function.dfg.instruction_results(instruction_id); - results.iter().all(|result| !self.used_values.contains(result)) + let results_unused = results.iter().all(|result| !self.used_values.contains(result)); + results_unused && !function.dfg.is_returned_in_databus(instruction_id) } else if let Instruction::Call { func, arguments } = instruction { // TODO: make this more general for instructions which don't have results but have side effects "sometimes" like `Intrinsic::AsWitness` let as_witness_id = function.dfg.get_intrinsic(Intrinsic::AsWitness); @@ -1194,4 +1197,17 @@ mod test { } "#); } + + #[test] + fn keeps_unused_databus_return_value() { + let src = r#" + acir(inline) predicate_pure fn main f0 { + return_data: v0 + b0(): + v0 = make_array [Field 0] : [Field; 1] + unreachable + } + "#; + assert_ssa_does_not_change(src, Ssa::dead_instruction_elimination); + } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs index 34ba0dbde89..1b1492d17bd 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_unreachable_instructions.rs @@ -189,7 +189,17 @@ impl Function { } if current_block_reachability == Reachability::Unreachable { - context.remove_current_instruction(); + if context.dfg.is_returned_in_databus(context.instruction_id) { + // We have to keep databus assignments at the end of the ACIR main function alive, + // otherwise we can't print the SSA, as it will crash trying to normalize values + // that no longer get created in the SSA. + // The reason it is enough to this only for unreachable blocks without worrying + // about their successors is that databus is only used in ACIR, and we only remove + // unreachable instructions after flattening, so there is only one block. + remove_and_replace_with_defaults(context, func_id, block_id); + } else { + context.remove_current_instruction(); + } return; } @@ -1375,4 +1385,32 @@ mod test { } "#); } + + #[test] + fn replaces_databus_return_data_with_default_in_unreachable() { + let src = " + acir(inline) predicate_pure fn main f0 { + return_data: v3 + b0(v0: u32): + constrain u1 0 == u1 1 + v1 = sub v0, u32 10 + v2 = cast v1 as Field + v3 = make_array [v2] : [Field; 1] + return v3 + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.remove_unreachable_instructions(); + + assert_ssa_snapshot!(ssa, @r" + acir(inline) predicate_pure fn main f0 { + return_data: v4 + b0(v0: u32): + constrain u1 0 == u1 1 + v4 = make_array [Field 0] : [Field; 1] + unreachable + } + "); + } }