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

chore: make eval_constants a pure function #3027

Merged
merged 5 commits into from
Oct 9, 2023
Merged
Changes from 2 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
82 changes: 41 additions & 41 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,15 +646,11 @@
let operand_type = dfg.type_of_value(self.lhs);

if let (Some(lhs), Some(rhs)) = (lhs, rhs) {
// If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.
// Thus, we do not evaluate this divison as we want to avoid triggering a panic,
// and division by zero should be handled by laying down constraints during ACIR generation.
if matches!(self.operator, BinaryOp::Div | BinaryOp::Mod) && rhs == FieldElement::zero()
{
return SimplifyResult::None;
}
return match self.eval_constants(dfg, lhs, rhs, operand_type) {
Some(value) => SimplifyResult::SimplifiedTo(value),
return match eval_constant_binary_op(lhs, rhs, self.operator, operand_type) {
Some((result, result_type)) => {
let value = dfg.make_constant(result, result_type);
SimplifyResult::SimplifiedTo(value)
}
None => SimplifyResult::None,
};
}
Expand Down Expand Up @@ -775,47 +771,51 @@
}
SimplifyResult::None
}
}

/// Evaluate the two constants with the operation specified by self.operator.
/// Pushes the resulting value to the given DataFlowGraph's constants and returns it.
fn eval_constants(
&self,
dfg: &mut DataFlowGraph,
lhs: FieldElement,
rhs: FieldElement,
mut operand_type: Type,
) -> Option<Id<Value>> {
let value = match &operand_type {
Type::Numeric(NumericType::NativeField) => {
self.operator.get_field_function()?(lhs, rhs)
/// Evaluate a binary operation with constant arguments.
fn eval_constant_binary_op(
lhs: FieldElement,
rhs: FieldElement,
operator: BinaryOp,
mut operand_type: Type,
) -> Option<(FieldElement, Type)> {
let value = match &operand_type {
Type::Numeric(NumericType::NativeField) => {
// If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.

Check warning on line 785 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (divison)
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
// Thus, we do not evaluate the division in this method, as we want to avoid triggering a panic,
// and the operation should be handled by ACIR generation.
if matches!(operator, BinaryOp::Div | BinaryOp::Mod) && rhs == FieldElement::zero() {
return None;
}
Type::Numeric(NumericType::Unsigned { bit_size }) => {
let function = self.operator.get_u128_function();

let lhs = truncate(lhs.try_into_u128()?, *bit_size);
let rhs = truncate(rhs.try_into_u128()?, *bit_size);
operator.get_field_function()?(lhs, rhs)
}
Type::Numeric(NumericType::Unsigned { bit_size }) => {
let function = operator.get_u128_function();

// The divisor is being truncated into the type of the operand, which can potentially
// lead to the rhs being zero.
// If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.
// Thus, we do not evaluate the division in this method, as we want to avoid triggering a panic,
// and the operation should be handled by ACIR generation.
if matches!(self.operator, BinaryOp::Div) && rhs == 0 {
return None;
}
let lhs = truncate(lhs.try_into_u128()?, *bit_size);
let rhs = truncate(rhs.try_into_u128()?, *bit_size);

let result = function(lhs, rhs);
truncate(result, *bit_size).into()
// The divisor is being truncated into the type of the operand, which can potentially
// lead to the rhs being zero.
// If the rhs of a division is zero, attempting to evaluate the divison will cause a compiler panic.

Check warning on line 801 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (divison)
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
// Thus, we do not evaluate the division in this method, as we want to avoid triggering a panic,
// and the operation should be handled by ACIR generation.
if matches!(operator, BinaryOp::Div | BinaryOp::Mod) && rhs == 0 {
return None;
}
_ => return None,
};

if matches!(self.operator, BinaryOp::Eq | BinaryOp::Lt) {
operand_type = Type::bool();
let result = function(lhs, rhs);
truncate(result, *bit_size).into()
}
_ => return None,
};

Some(dfg.make_constant(value, operand_type))
if matches!(operator, BinaryOp::Eq | BinaryOp::Lt) {
operand_type = Type::bool();
}

Some((value, operand_type))
}

fn truncate(int: u128, bit_size: u32) -> u128 {
Expand Down