Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove redundant EnableSideEffects instructions #5440

Merged
Merged
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
100 changes: 97 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::ssa::{
dfg::DataFlowGraph,
function::Function,
instruction::{BinaryOp, Instruction, Intrinsic},
types::Type,
value::Value,
},
ssa_gen::Ssa,
Expand Down Expand Up @@ -62,6 +63,7 @@ impl Context {
) {
let instructions = function.dfg[block].take_instructions();

let mut active_condition = function.dfg.make_constant(FieldElement::one(), Type::bool());
let mut last_side_effects_enabled_instruction = None;

let mut new_instructions = Vec::with_capacity(instructions.len());
Expand All @@ -72,19 +74,26 @@ impl Context {
// instructions with side effects then we can drop the instruction we're holding and
// continue with the new `Instruction::EnableSideEffects`.
if let Instruction::EnableSideEffects { condition } = instruction {
// If this instruction isn't changing the currently active condition then we can ignore it.
if active_condition == *condition {
continue;
}

// If we're seeing an `enable_side_effects u1 1` then we want to insert it immediately.
// This is because we want to maximize the effect it will have.
if function
let condition_is_one = function
.dfg
.get_numeric_constant(*condition)
.map_or(false, |condition| condition.is_one())
{
.map_or(false, |condition| condition.is_one());
if condition_is_one {
new_instructions.push(instruction_id);
last_side_effects_enabled_instruction = None;
active_condition = *condition;
continue;
}

last_side_effects_enabled_instruction = Some(instruction_id);
active_condition = *condition;
continue;
}

Expand Down Expand Up @@ -172,3 +181,88 @@ impl Context {
}
}
}

#[cfg(test)]
mod test {

use crate::ssa::{
function_builder::FunctionBuilder,
ir::{
instruction::{BinaryOp, Instruction},
map::Id,
types::Type,
},
};

#[test]
fn remove_chains_of_same_condition() {
// acir(inline) fn main f0 {
// b0(v0: Field):
// enable_side_effects u1 1
// v4 = mul v0, Field 2
// enable_side_effects u1 1
// v5 = mul v0, Field 2
// enable_side_effects u1 1
// v6 = mul v0, Field 2
// enable_side_effects u1 1
// v7 = mul v0, Field 2
// enable_side_effects u1 1
// (no terminator instruction)
// }
//
// After constructing this IR, we run constant folding which should replace the second cast
// with a reference to the results to the first. This then allows us to optimize away
// the constrain instruction as both inputs are known to be equal.
//
// The first cast instruction is retained and will be removed in the dead instruction elimination pass.
let main_id = Id::test_new(0);

// Compiling main
let mut builder = FunctionBuilder::new("main".into(), main_id);
let v0 = builder.add_parameter(Type::field());

let two = builder.numeric_constant(2u128, Type::field());

let one = builder.numeric_constant(1u128, Type::bool());

builder.insert_enable_side_effects_if(one);
builder.insert_binary(v0, BinaryOp::Mul, two);
builder.insert_enable_side_effects_if(one);
builder.insert_binary(v0, BinaryOp::Mul, two);
builder.insert_enable_side_effects_if(one);
builder.insert_binary(v0, BinaryOp::Mul, two);
builder.insert_enable_side_effects_if(one);
builder.insert_binary(v0, BinaryOp::Mul, two);
builder.insert_enable_side_effects_if(one);

let ssa = builder.finish();

println!("{ssa}");

let main = ssa.main();
let instructions = main.dfg[main.entry_block()].instructions();
assert_eq!(instructions.len(), 9);

// Expected output:
//
// acir(inline) fn main f0 {
// b0(v0: Field):
// v3 = mul v0, Field 2
// v4 = mul v0, Field 2
// v5 = mul v0, Field 2
// v6 = mul v0, Field 2
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
// (no terminator instruction)
// }
let ssa = ssa.remove_enable_side_effects();

println!("{ssa}");

let main = ssa.main();
let instructions = main.dfg[main.entry_block()].instructions();

assert_eq!(instructions.len(), 4);
for instruction in instructions.iter().take(4) {
assert_eq!(&main.dfg[*instruction], &Instruction::binary(BinaryOp::Mul, v0, two));
}
}
}
Loading