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.

Sign up for GitHub

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

Jump to bottom

feat: Remove an unnecessary witness in mul_with_witness #2078

Merged
merged 1 commit into from
Jul 28, 2023
Merged

feat: Remove an unnecessary witness in mul_with_witness #2078

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 11 additions & 7 deletions crates/noirc_evaluator/src/ssa_refactor/acir_gen/acir_ir/generated_acir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ impl GeneratedAcir {
/// If one has multiplicative term and the other is of degree one or more,
/// the function creates [intermediate variables][`Witness`] accordingly.
/// There are two cases where we can optimize the multiplication between two expressions:
/// 1. If both expressions have at most a total degree of 1 in each term, then we can just multiply them
/// 1. If the sum of the degrees of both expressions is at most 2, then we can just multiply them
/// as each term in the result will be degree-2.
/// 2. If one expression is a constant, then we can just multiply the constant with the other expression
///
Expand All @@ -268,10 +268,14 @@ impl GeneratedAcir {
let lhs_is_linear = lhs.is_linear();
let rhs_is_linear = rhs.is_linear();

// Case 1: Both expressions have at most a total degree of 1 in each term
if lhs_is_linear && rhs_is_linear {
return (lhs * rhs)
.expect("one of the expressions is a constant and so this should not fail");
// Case 1: The sum of the degrees of both expressions is at most 2.
//
// If one of the expressions is constant then it does not increase the degree when multiplying by another expression.
// If both of the expressions are linear (degree <=1) then the product will be at most degree 2.
let both_are_linear = lhs_is_linear && rhs_is_linear;
let either_is_const = lhs.is_const() || rhs.is_const();
if both_are_linear || either_is_const {
return (lhs * rhs).expect("Both expressions are degree <= 1");
}

// Case 2: One or both of the sides needs to be reduced to a degree-1 univariate polynomial
Expand All @@ -285,7 +289,7 @@ impl GeneratedAcir {
// rhs, we only need to square the lhs.
if lhs == rhs {
return (&*lhs_reduced * &*lhs_reduced)
.expect("Both expressions are reduced to be degree<=1");
.expect("Both expressions are reduced to be degree <= 1");
};

let rhs_reduced = if rhs_is_linear {
Expand All @@ -294,7 +298,7 @@ impl GeneratedAcir {
Cow::Owned(self.get_or_create_witness(rhs).into())
};

(&*lhs_reduced * &*rhs_reduced).expect("Both expressions are reduced to be degree<=1")
(&*lhs_reduced * &*rhs_reduced).expect("Both expressions are reduced to be degree <= 1")
}

/// Signed division lhs / rhs
Expand Down