Skip to content

Commit

Permalink
feat: remove redundant EnableSideEffects instructions (#5440)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

We currently pick up quite a lot of duplicated `EnableSideEffects`
instructions as we perform optimizations on the SSA which tends to
clutter up the output. This PR adds a small change to remember the
currently active predicate condition and skips any instructions which
will just will just reactivate the current predicate.

I've added a test case which shows the sort of SSA which we want to
avoid.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored Jul 9, 2024
1 parent d44f882 commit e153ecb
Showing 1 changed file with 97 additions and 3 deletions.
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
// (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));
}
}
}

0 comments on commit e153ecb

Please sign in to comment.