Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<InstructionId> for DataFlowGraph {
Expand Down
18 changes: 17 additions & 1 deletion compiler/noirc_evaluator/src/ssa/opt/die.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
}
");
}
}
Loading