Skip to content

Commit

Permalink
chore: Avoid duplicate Not instructions during flattening (#6886)
Browse files Browse the repository at this point in the history
  • Loading branch information
jfecher authored Dec 19, 2024
1 parent 8abc53a commit 9108df3
Showing 1 changed file with 43 additions and 31 deletions.
74 changes: 43 additions & 31 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,13 @@ struct Context<'f> {
///
/// The `ValueId` here is that which is returned by the allocate instruction.
local_allocations: HashSet<ValueId>,

/// A map from `cond` to `Not(cond)`
///
/// `Not` instructions are inserted constantly by this pass and this map helps keep
/// us from unnecessarily inserting extra instructions, and keeps ids unique which
/// helps simplifications.
not_instructions: HashMap<ValueId, ValueId>,
}

#[derive(Clone)]
Expand Down Expand Up @@ -256,6 +263,7 @@ fn flatten_function_cfg(function: &mut Function, no_predicates: &HashMap<Functio
condition_stack: Vec::new(),
arguments_stack: Vec::new(),
local_allocations: HashSet::default(),
not_instructions: HashMap::default(),
};
context.flatten(no_predicates);
}
Expand Down Expand Up @@ -443,8 +451,8 @@ impl<'f> Context<'f> {

let condition_call_stack =
self.inserter.function.dfg.get_value_call_stack_id(cond_context.condition);
let else_condition =
self.insert_instruction(Instruction::Not(cond_context.condition), condition_call_stack);

let else_condition = self.not_instruction(cond_context.condition, condition_call_stack);
let else_condition = self.link_condition(else_condition);

let old_allocations = std::mem::take(&mut self.local_allocations);
Expand All @@ -464,6 +472,16 @@ impl<'f> Context<'f> {
vec![self.cfg.successors(*block).next().unwrap()]
}

fn not_instruction(&mut self, condition: ValueId, call_stack: CallStackId) -> ValueId {
if let Some(existing) = self.not_instructions.get(&condition) {
return *existing;
}

let not = self.insert_instruction(Instruction::Not(condition), call_stack);
self.not_instructions.insert(condition, not);
not
}

/// Process the 'exit' block of a conditional statement
fn else_stop(&mut self, block: &BasicBlockId) -> Vec<BasicBlockId> {
let mut cond_context = self.condition_stack.pop().unwrap();
Expand Down Expand Up @@ -671,8 +689,7 @@ impl<'f> Context<'f> {
.insert_instruction_with_typevars(load, Some(vec![typ]), call_stack)
.first();

let else_condition =
self.insert_instruction(Instruction::Not(condition), call_stack);
let else_condition = self.not_instruction(condition, call_stack);

let instruction = Instruction::IfElse {
then_condition: condition,
Expand Down Expand Up @@ -788,7 +805,7 @@ impl<'f> Context<'f> {
fn var_or_one(&mut self, var: ValueId, condition: ValueId, call_stack: CallStackId) -> ValueId {
let field =
self.insert_instruction(Instruction::binary(BinaryOp::Mul, var, condition), call_stack);
let not_condition = self.insert_instruction(Instruction::Not(condition), call_stack);
let not_condition = self.not_instruction(condition, call_stack);
self.insert_instruction(
Instruction::binary(BinaryOp::Add, field, not_condition),
call_stack,
Expand Down Expand Up @@ -909,7 +926,6 @@ mod test {
v8 = mul v5, v2
v9 = add v7, v8
store v9 at v1
v10 = not v0
enable_side_effects u1 1
return
}
Expand Down Expand Up @@ -948,15 +964,14 @@ mod test {
v8 = mul v5, v2
v9 = add v7, v8
store v9 at v1
v10 = not v0
enable_side_effects v10
v11 = load v1 -> Field
v12 = cast v10 as Field
v13 = cast v0 as Field
v15 = mul v12, Field 6
v16 = mul v13, v11
v17 = add v15, v16
store v17 at v1
enable_side_effects v3
v10 = load v1 -> Field
v11 = cast v3 as Field
v12 = cast v0 as Field
v14 = mul v11, Field 6
v15 = mul v12, v10
v16 = add v14, v15
store v16 at v1
enable_side_effects u1 1
return
}
Expand Down Expand Up @@ -1084,16 +1099,14 @@ mod test {
v23 = mul v20, Field 6
v24 = mul v21, v16
v25 = add v23, v24
enable_side_effects v0
v26 = not v0
enable_side_effects v26
v27 = cast v26 as Field
v28 = cast v0 as Field
v30 = mul v27, Field 3
v31 = mul v28, v25
v32 = add v30, v31
enable_side_effects v3
v26 = cast v3 as Field
v27 = cast v0 as Field
v29 = mul v26, Field 3
v30 = mul v27, v25
v31 = add v29, v30
enable_side_effects u1 1
return v32
return v31
}";

let main = ssa.main();
Expand Down Expand Up @@ -1287,13 +1300,12 @@ mod test {
v16 = mul v14, v11
v17 = add v15, v16
store v17 at v6
v18 = not v5
enable_side_effects v18
v19 = load v6 -> u8
v20 = cast v18 as u8
v21 = cast v4 as u8
v22 = mul v21, v19
store v22 at v6
enable_side_effects v12
v18 = load v6 -> u8
v19 = cast v12 as u8
v20 = cast v4 as u8
v21 = mul v20, v18
store v21 at v6
enable_side_effects u1 1
constrain v5 == u1 1
return
Expand Down

0 comments on commit 9108df3

Please sign in to comment.