Skip to content

Commit

Permalink
fix(ssa refactor): Add missing calls to resolve in Instruction::simpl…
Browse files Browse the repository at this point in the history
…ify (#1678)

Fix missing calls to resolve in Instruction::simplify
  • Loading branch information
jfecher authored Jun 13, 2023
1 parent 7ce8cce commit 07b07d0
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 8 deletions.
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ impl DataFlowGraph {
&self,
value: ValueId,
) -> Option<(FieldElement, Type)> {
match &self.values[value] {
match &self.values[self.resolve(value)] {
Value::NumericConstant { constant, typ } => Some((*constant, typ.clone())),
_ => None,
}
Expand All @@ -320,7 +320,7 @@ impl DataFlowGraph {
&self,
value: ValueId,
) -> Option<(im::Vector<ValueId>, Rc<CompositeType>)> {
match &self.values[value] {
match &self.values[self.resolve(value)] {
// Vectors are shared, so cloning them is cheap
Value::Array { array, element_type } => Some((array.clone(), element_type.clone())),
_ => None,
Expand Down
8 changes: 4 additions & 4 deletions crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ impl Instruction {
}
}
Instruction::Not(value) => {
match &dfg[*value] {
match &dfg[dfg.resolve(*value)] {
// Limit optimizing ! on constants to only booleans. If we tried it on fields,
// there is no Not on FieldElement, so we'd need to convert between u128. This
// would be incorrect however since the extra bits on the field would not be flipped.
Expand Down Expand Up @@ -497,13 +497,13 @@ impl Binary {
}
}
BinaryOp::Eq => {
if self.lhs == self.rhs {
if dfg.resolve(self.lhs) == dfg.resolve(self.rhs) {
let one = dfg.make_constant(FieldElement::one(), Type::bool());
return SimplifyResult::SimplifiedTo(one);
}
}
BinaryOp::Lt => {
if self.lhs == self.rhs {
if dfg.resolve(self.lhs) == dfg.resolve(self.rhs) {
let zero = dfg.make_constant(FieldElement::zero(), Type::bool());
return SimplifyResult::SimplifiedTo(zero);
}
Expand All @@ -523,7 +523,7 @@ impl Binary {
}
}
BinaryOp::Xor => {
if self.lhs == self.rhs {
if dfg.resolve(self.lhs) == dfg.resolve(self.rhs) {
let zero = dfg.make_constant(FieldElement::zero(), Type::bool());
return SimplifyResult::SimplifiedTo(zero);
}
Expand Down
12 changes: 10 additions & 2 deletions crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,11 @@ impl<'f> Context<'f> {
if destination == self.branch_ends[&jmpif_block] {
// If the branch destination is the same as the end of the branch, this must be the
// 'else' case of an if with no else - so there is no else branch.
Branch { condition: new_condition, last_block: jmpif_block, store_values: HashMap::new() }
Branch {
condition: new_condition,
last_block: jmpif_block,
store_values: HashMap::new(),
}
} else {
self.push_condition(jmpif_block, new_condition);
self.insert_current_side_effects_enabled();
Expand All @@ -459,7 +463,11 @@ impl<'f> Context<'f> {
self.conditions.pop();
let stores_in_branch = std::mem::replace(&mut self.store_values, old_stores);

Branch { condition: new_condition, last_block: final_block, store_values: stores_in_branch }
Branch {
condition: new_condition,
last_block: final_block,
store_values: stores_in_branch,
}
}
}

Expand Down

0 comments on commit 07b07d0

Please sign in to comment.