From da7de2ea6b30a03c403c95a49ce1036bd63bfe19 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 2 Jan 2024 12:40:28 +0000 Subject: [PATCH] feat: remove unnecessary predicate from `lt` instruction --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 29 +++++++++--------- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 30 +++++-------------- 2 files changed, 22 insertions(+), 37 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index db2e24af14..c2441d989b 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -991,23 +991,25 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_count: u32, - predicate: AcirVar, ) -> Result { let pow_last = self.add_constant(FieldElement::from(1_u128 << (bit_count - 1))); let pow = self.add_constant(FieldElement::from(1_u128 << (bit_count))); // We check whether the inputs have same sign or not by computing the XOR of their bit sign + + // Predicate is always active as `pow_last` is known to be non-zero. + let one = self.add_constant(1_u128); let lhs_sign = self.div_var( lhs, pow_last, AcirType::NumericType(NumericType::Unsigned { bit_size: bit_count }), - predicate, + one, )?; let rhs_sign = self.div_var( rhs, pow_last, AcirType::NumericType(NumericType::Unsigned { bit_size: bit_count }), - predicate, + one, )?; let same_sign = self.xor_var( lhs_sign, @@ -1020,7 +1022,7 @@ impl AcirContext { let diff = self.sub_var(no_underflow, rhs)?; // We check the 'bit sign' of the difference - let diff_sign = self.less_than_var(diff, pow, bit_count + 1, predicate)?; + let diff_sign = self.less_than_var(diff, pow, bit_count + 1)?; // Then the result is simply diff_sign XOR same_sign (can be checked with a truth table) self.xor_var( @@ -1037,7 +1039,6 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, max_bits: u32, - predicate: AcirVar, ) -> Result { // Returns a `Witness` that is constrained to be: // - `1` if lhs >= rhs @@ -1062,6 +1063,7 @@ impl AcirContext { // // TODO: perhaps this should be a user error, instead of an assert assert!(max_bits + 1 < FieldElement::max_num_bits()); + let two_max_bits = self .add_constant(FieldElement::from(2_i128).pow(&FieldElement::from(max_bits as i128))); let diff = self.sub_var(lhs, rhs)?; @@ -1091,13 +1093,11 @@ impl AcirContext { // let k = b - a // - 2^{max_bits} - k == q * 2^{max_bits} + r // - This is only the case when q == 0 and r == 2^{max_bits} - k - // - let (q, _) = self.euclidean_division_var( - comparison_evaluation, - two_max_bits, - max_bits + 1, - predicate, - )?; + + // Predicate is always active as we know `two_max_bits` is always non-zero. + let one = self.add_constant(1_u128); + let (q, _) = + self.euclidean_division_var(comparison_evaluation, two_max_bits, max_bits + 1, one)?; Ok(q) } @@ -1108,11 +1108,10 @@ impl AcirContext { lhs: AcirVar, rhs: AcirVar, bit_size: u32, - predicate: AcirVar, ) -> Result { // Flip the result of calling more than equal method to // compute less than. - let comparison = self.more_than_eq_var(lhs, rhs, bit_size, predicate)?; + let comparison = self.more_than_eq_var(lhs, rhs, bit_size)?; let one = self.add_constant(FieldElement::one()); self.sub_var(one, comparison) // comparison_negated @@ -1508,7 +1507,7 @@ impl AcirContext { bit_size: u32, predicate: AcirVar, ) -> Result<(), RuntimeError> { - let lhs_less_than_rhs = self.more_than_eq_var(rhs, lhs, bit_size, predicate)?; + let lhs_less_than_rhs = self.more_than_eq_var(rhs, lhs, bit_size)?; self.maybe_eq_predicate(lhs_less_than_rhs, predicate) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 6a48dadaf3..e5f5d23d54 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -1571,15 +1571,10 @@ impl Context { // this Eq instruction is being used for a constrain statement BinaryOp::Eq => self.acir_context.eq_var(lhs, rhs), BinaryOp::Lt => match binary_type { - AcirType::NumericType(NumericType::Signed { .. }) => self - .acir_context - .less_than_signed(lhs, rhs, bit_count, self.current_side_effects_enabled_var), - _ => self.acir_context.less_than_var( - lhs, - rhs, - bit_count, - self.current_side_effects_enabled_var, - ), + AcirType::NumericType(NumericType::Signed { .. }) => { + self.acir_context.less_than_signed(lhs, rhs, bit_count) + } + _ => self.acir_context.less_than_var(lhs, rhs, bit_count), }, BinaryOp::Xor => self.acir_context.xor_var(lhs, rhs, binary_type), BinaryOp::And => self.acir_context.and_var(lhs, rhs, binary_type), @@ -2141,19 +2136,11 @@ impl Context { let current_index = self.acir_context.add_constant(i); // Check that we are above the lower bound of the insertion index - let greater_eq_than_idx = self.acir_context.more_than_eq_var( - current_index, - flat_user_index, - 64, - self.current_side_effects_enabled_var, - )?; + let greater_eq_than_idx = + self.acir_context.more_than_eq_var(current_index, flat_user_index, 64)?; // Check that we are below the upper bound of the insertion index - let less_than_idx = self.acir_context.less_than_var( - current_index, - max_flat_user_index, - 64, - self.current_side_effects_enabled_var, - )?; + let less_than_idx = + self.acir_context.less_than_var(current_index, max_flat_user_index, 64)?; // Read from the original slice the value we want to insert into our new slice. // We need to make sure that we read the previous element when our current index is greater than insertion index. @@ -2328,7 +2315,6 @@ impl Context { current_index, flat_user_index, 64, - self.current_side_effects_enabled_var, )?; let shifted_value_pred =