Merge branch 'master' into tf/function-exports

acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs

@@ -48,25 +48,51 @@ impl RangeOptimizer {
     /// only store the fact that we have constrained it to
     /// be 16 bits.
     fn collect_ranges(circuit: &Circuit) -> BTreeMap<Witness, u32> {
-        let mut witness_to_bit_sizes = BTreeMap::new();
+        let mut witness_to_bit_sizes: BTreeMap<Witness, u32> = BTreeMap::new();
 
         for opcode in &circuit.opcodes {
-            // Extract the witness index and number of bits,
-            // if it is a range constraint
-            let (witness, num_bits) = match extract_range_opcode(opcode) {
-                Some(func_inputs) => func_inputs,
-                None => continue,
+            let Some((witness, num_bits)) = (match opcode {
+                Opcode::AssertZero(expr) => {
+                    // If the opcode is constraining a witness to be equal to a value then it can be considered
+                    // as a range opcode for the number of bits required to hold that value.
+                    if expr.is_degree_one_univariate() {
+                        let (k, witness) = expr.linear_combinations[0];
+                        let constant = expr.q_c;
+                        let witness_value = -constant / k;
+
+                        if witness_value.is_zero() {
+                            Some((witness, 0))
+                        } else {
+                            // We subtract off 1 bit from the implied witness value to give the weakest range constraint
+                            // which would be stricter than the constraint imposed by this opcode.
+                            let implied_range_constraint_bits = witness_value.num_bits() - 1;
+                            Some((witness, implied_range_constraint_bits))
+                        }
+                    } else {
+                        None
+                    }
+                }
+
+
+                Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
+                    input: FunctionInput { witness, num_bits },
+                }) => {
+                    Some((*witness, *num_bits))
+                }
+
+                _ => None,
+            }) else {
+                continue;
             };
 
             // Check if the witness has already been recorded and if the witness
             // size is more than the current one, we replace it
-            let should_replace = match witness_to_bit_sizes.get(&witness).copied() {
-                Some(old_range_bits) => old_range_bits > num_bits,
-                None => true,
-            };
-            if should_replace {
-                witness_to_bit_sizes.insert(witness, num_bits);
-            }
+            witness_to_bit_sizes
+                .entry(witness)
+                .and_modify(|old_range_bits| {
+                    *old_range_bits = std::cmp::min(*old_range_bits, num_bits);
+                })
+                .or_insert(num_bits);
         }
         witness_to_bit_sizes
     }
@@ -116,16 +142,10 @@ impl RangeOptimizer {
 /// Extract the range opcode from the `Opcode` enum
 /// Returns None, if `Opcode` is not the range opcode.
 fn extract_range_opcode(opcode: &Opcode) -> Option<(Witness, u32)> {
-    // Range constraints are blackbox function calls
-    // so we first extract the function call
-    let func_call = match opcode {
-        acir::circuit::Opcode::BlackBoxFuncCall(func_call) => func_call,
-        _ => return None,
-    };
-
-    // Skip if it is not a range constraint
-    match func_call {
-        BlackBoxFuncCall::RANGE { input } => Some((input.witness, input.num_bits)),
+    match opcode {
+        Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => {
+            Some((input.witness, input.num_bits))
+        }
         _ => None,
     }
 }
@@ -246,4 +266,17 @@ mod tests {
         let (optimized_circuit, _) = optimizer.replace_redundant_ranges(acir_opcode_positions);
         assert_eq!(optimized_circuit.opcodes.len(), 5);
     }
+
+    #[test]
+    fn constant_implied_ranges() {
+        // The optimizer should use knowledge about constant witness assignments to remove range opcodes.
+        let mut circuit = test_circuit(vec![(Witness(1), 16)]);
+
+        circuit.opcodes.push(Opcode::AssertZero(Witness(1).into()));
+        let acir_opcode_positions = circuit.opcodes.iter().enumerate().map(|(i, _)| i).collect();
+        let optimizer = RangeOptimizer::new(circuit);
+        let (optimized_circuit, _) = optimizer.replace_redundant_ranges(acir_opcode_positions);
+        assert_eq!(optimized_circuit.opcodes.len(), 1);
+        assert_eq!(optimized_circuit.opcodes[0], Opcode::AssertZero(Witness(1).into()));
+    }
 }

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs

@@ -92,6 +92,28 @@ pub(crate) fn convert_black_box_call(
                 )
             }
         }
+        BlackBoxFunc::EcdsaSecp256r1 => {
+            if let (
+                [BrilligVariable::BrilligArray(public_key_x), BrilligVariable::BrilligArray(public_key_y), BrilligVariable::BrilligArray(signature), message],
+                [BrilligVariable::Simple(result_register)],
+            ) = (function_arguments, function_results)
+            {
+                let message_hash_vector =
+                    convert_array_or_vector(brillig_context, message, bb_func);
+                brillig_context.black_box_op_instruction(BlackBoxOp::EcdsaSecp256r1 {
+                    hashed_msg: message_hash_vector.to_heap_vector(),
+                    public_key_x: public_key_x.to_heap_array(),
+                    public_key_y: public_key_y.to_heap_array(),
+                    signature: signature.to_heap_array(),
+                    result: *result_register,
+                });
+            } else {
+                unreachable!(
+                    "ICE: EcdsaSecp256r1 expects four array arguments and one register result"
+                )
+            }
+        }
+
         BlackBoxFunc::PedersenCommitment => {
             if let (
                 [message, BrilligVariable::Simple(domain_separator)],
@@ -160,7 +182,18 @@ pub(crate) fn convert_black_box_call(
                 )
             }
         }
-        _ => unimplemented!("ICE: Black box function {:?} is not implemented", bb_func),
+        BlackBoxFunc::AND => {
+            unreachable!("ICE: `BlackBoxFunc::AND` calls should be transformed into a `BinaryOp`")
+        }
+        BlackBoxFunc::XOR => {
+            unreachable!("ICE: `BlackBoxFunc::XOR` calls should be transformed into a `BinaryOp`")
+        }
+        BlackBoxFunc::RANGE => unreachable!(
+            "ICE: `BlackBoxFunc::RANGE` calls should be transformed into a `Instruction::Cast`"
+        ),
+        BlackBoxFunc::RecursiveAggregation => unimplemented!(
+            "ICE: `BlackBoxFunc::RecursiveAggregation` is not implemented by the Brillig VM"
+        ),
     }
 }
 

compiler/noirc_evaluator/src/errors.rs

@@ -106,7 +106,7 @@ pub enum InternalError {
     #[error("ICE: Undeclared AcirVar")]
     UndeclaredAcirVar { call_stack: CallStack },
     #[error("ICE: Expected {expected:?}, found {found:?}")]
-    UnExpected { expected: String, found: String, call_stack: CallStack },
+    Unexpected { expected: String, found: String, call_stack: CallStack },
 }
 
 impl RuntimeError {
@@ -119,7 +119,7 @@ impl RuntimeError {
                 | InternalError::MissingArg { call_stack, .. }
                 | InternalError::NotAConstant { call_stack, .. }
                 | InternalError::UndeclaredAcirVar { call_stack }
-                | InternalError::UnExpected { call_stack, .. },
+                | InternalError::Unexpected { call_stack, .. },
             )
             | RuntimeError::FailedConstraint { call_stack, .. }
             | RuntimeError::IndexOutOfBounds { call_stack, .. }

compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -302,7 +302,7 @@ impl Context {
                     let len = if matches!(typ, Type::Array(_, _)) {
                         typ.flattened_size()
                     } else {
-                        return Err(InternalError::UnExpected {
+                        return Err(InternalError::Unexpected {
                             expected: "Block params should be an array".to_owned(),
                             found: format!("Instead got {:?}", typ),
                             call_stack: self.acir_context.get_call_stack(),
@@ -640,7 +640,7 @@ impl Context {
             Instruction::ArrayGet { array, index } => (array, index, None),
             Instruction::ArraySet { array, index, value, .. } => (array, index, Some(value)),
             _ => {
-                return Err(InternalError::UnExpected {
+                return Err(InternalError::Unexpected {
                     expected: "Instruction should be an ArrayGet or ArraySet".to_owned(),
                     found: format!("Instead got {:?}", dfg[instruction]),
                     call_stack: self.acir_context.get_call_stack(),
@@ -697,7 +697,7 @@ impl Context {
 
         match self.convert_value(array, dfg) {
             AcirValue::Var(acir_var, _) => {
-                return Err(RuntimeError::InternalError(InternalError::UnExpected {
+                return Err(RuntimeError::InternalError(InternalError::Unexpected {
                     expected: "an array value".to_string(),
                     found: format!("{acir_var:?}"),
                     call_stack: self.acir_context.get_call_stack(),
@@ -788,7 +788,7 @@ impl Context {
                 let slice_sizes = if store_type.contains_slice_element() {
                     self.compute_slice_sizes(store, None, dfg);
                     self.slice_sizes.get(&store).cloned().ok_or_else(|| {
-                        InternalError::UnExpected {
+                        InternalError::Unexpected {
                             expected: "Store value should have slice sizes computed".to_owned(),
                             found: "Missing key in slice sizes map".to_owned(),
                             call_stack: self.acir_context.get_call_stack(),
@@ -1013,7 +1013,7 @@ impl Context {
         let array = match dfg[instruction] {
             Instruction::ArraySet { array, .. } => array,
             _ => {
-                return Err(InternalError::UnExpected {
+                return Err(InternalError::Unexpected {
                     expected: "Instruction should be an ArraySet".to_owned(),
                     found: format!("Instead got {:?}", dfg[instruction]),
                     call_stack: self.acir_context.get_call_stack(),
@@ -1235,7 +1235,7 @@ impl Context {
                                 }
                             }
                             _ => {
-                                return Err(InternalError::UnExpected {
+                                return Err(InternalError::Unexpected {
                                     expected: "AcirValue::DynamicArray or AcirValue::Array"
                                         .to_owned(),
                                     found: format!("{:?}", array_acir_value),
@@ -1246,7 +1246,7 @@ impl Context {
                         }
                     }
                     _ => {
-                        return Err(InternalError::UnExpected {
+                        return Err(InternalError::Unexpected {
                             expected: "array or instruction".to_owned(),
                             found: format!("{:?}", &dfg[array_id]),
                             call_stack: self.acir_context.get_call_stack(),
@@ -1256,7 +1256,7 @@ impl Context {
                 };
             }
             _ => {
-                return Err(InternalError::UnExpected {
+                return Err(InternalError::Unexpected {
                     expected: "array or slice".to_owned(),
                     found: array_typ.to_string(),
                     call_stack: self.acir_context.get_call_stack(),
@@ -1513,12 +1513,12 @@ impl Context {
     ) -> Result<AcirVar, InternalError> {
         match self.convert_value(value_id, dfg) {
             AcirValue::Var(acir_var, _) => Ok(acir_var),
-            AcirValue::Array(array) => Err(InternalError::UnExpected {
+            AcirValue::Array(array) => Err(InternalError::Unexpected {
                 expected: "a numeric value".to_string(),
                 found: format!("{array:?}"),
                 call_stack: self.acir_context.get_call_stack(),
             }),
-            AcirValue::DynamicArray(_) => Err(InternalError::UnExpected {
+            AcirValue::DynamicArray(_) => Err(InternalError::Unexpected {
                 expected: "a numeric value".to_string(),
                 found: "an array".to_string(),
                 call_stack: self.acir_context.get_call_stack(),