feat: Sync from noir (#6801)

Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
feat: place return value witnesses directly after function arguments
(noir-lang/noir#5142)
fix(experimental elaborator): Fix frontend tests when `--use-elaborator`
flag is specified (noir-lang/noir#5145)
chore: Remove unused `new_variables` argument from `resolve_type_inner`
(noir-lang/noir#5148)
chore: Remove hir to ast pass
(noir-lang/noir#5147)
fix(experimental elaborator): Fix definition kind of globals and tuple
patterns with `--use-elaborator` flag
(noir-lang/noir#5139)
chore: default to using bn254 in `noirc_frontend`
(noir-lang/noir#5144)
fix(experimental elaborator): Fix `impl Trait` when `--use-elaborator`
is selected (noir-lang/noir#5138)
fix: wrapping in signed division
(noir-lang/noir#5134)
chore(ci): don't raise MSRV issue if workflow cancelled
(noir-lang/noir#5143)
fix: use predicate for curve operations
(noir-lang/noir#5076)
fix(experimental elaborator): Fix global values used in the elaborator
(noir-lang/noir#5135)
fix(experimental elaborator): Clear generics after elaborating type
aliases (noir-lang/noir#5136)
fix(frontend): Resolve object types from method calls a single time
(noir-lang/noir#5131)
feat: Separate runtimes of SSA functions before inlining
(noir-lang/noir#5121)
chore: run all test programs in brillig as well as ACIR
(noir-lang/noir#5128)
END_COMMIT_OVERRIDE

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: Tom French <tom@tomfren.ch>
Co-authored-by: guipublic <47281315+guipublic@users.noreply.github.com>
Co-authored-by: guipublic <guipublic@gmail.com>
Co-authored-by: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com>

.noir-sync-commit

@@ -1 +1,2 @@
-f03f8aed34393f7b0f6f68a189ce6c6192f6af6e
+1252b5fcc7ed56bb55e95745b83be6e556805397
+

barretenberg/cpp/src/barretenberg/dsl/acir_format/ec_operations.cpp

@@ -35,10 +35,10 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val
     cycle_group_ct input2_point(x2, y2, infinite2);
     // Addition
     cycle_group_ct result = input1_point + input2_point;
-
-    auto x_normalized = result.x.normalize();
-    auto y_normalized = result.y.normalize();
-    auto infinite = result.is_point_at_infinity().normalize();
+    cycle_group_ct standard_result = result.get_standard_form();
+    auto x_normalized = standard_result.x.normalize();
+    auto y_normalized = standard_result.y.normalize();
+    auto infinite = standard_result.is_point_at_infinity().normalize();
     builder.assert_equal(x_normalized.witness_index, input.result_x);
     builder.assert_equal(y_normalized.witness_index, input.result_y);
     builder.assert_equal(infinite.witness_index, input.result_infinite);

barretenberg/cpp/src/barretenberg/dsl/acir_format/multi_scalar_mul.cpp

@@ -36,7 +36,7 @@ template <typename Builder> void create_multi_scalar_mul_constraint(Builder& bui
     }
 
     // Call batch_mul to multiply the points and scalars and sum the results
-    auto output_point = cycle_group_ct::batch_mul(points, scalars);
+    auto output_point = cycle_group_ct::batch_mul(points, scalars).get_standard_form();
 
     // Add the constraints
     builder.assert_equal(output_point.x.get_witness_index(), input.out_point_x);

noir/noir-repo/.github/workflows/test-rust-workspace-msrv.yml

@@ -111,7 +111,7 @@ jobs:
           fi
         env:
           # We treat any cancelled, skipped or failing jobs as a failure for the workflow as a whole.
-          FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}
+          FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'skipped') }}
 
       - name: Checkout
         if: ${{ failure() }}

noir/noir-repo/acvm-repo/brillig_vm/src/memory.rs

@@ -294,6 +294,13 @@ impl<F: AcirField> Memory<F> {
     }
 
     pub fn read_slice(&self, addr: MemoryAddress, len: usize) -> &[MemoryValue<F>] {
+        // Allows to read a slice of uninitialized memory if the length is zero.
+        // Ideally we'd be able to read uninitialized memory in general (as read does)
+        // but that's not possible if we want to return a slice instead of owned data.
+        if len == 0 {
+            return &[];
+        }
+
         &self.inner[addr.to_usize()..(addr.to_usize() + len)]
     }
 

noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs

@@ -58,8 +58,9 @@ pub(crate) fn optimize_into_acir(
     let ssa = SsaBuilder::new(program, print_passes, force_brillig_output, print_timings)?
         .run_pass(Ssa::defunctionalize, "After Defunctionalization:")
         .run_pass(Ssa::remove_paired_rc, "After Removing Paired rc_inc & rc_decs:")
-        .run_pass(Ssa::inline_functions, "After Inlining:")
+        .run_pass(Ssa::separate_runtime, "After Runtime Separation:")
         .run_pass(Ssa::resolve_is_unconstrained, "After Resolving IsUnconstrained:")
+        .run_pass(Ssa::inline_functions, "After Inlining:")
         // Run mem2reg with the CFG separated into blocks
         .run_pass(Ssa::mem2reg, "After Mem2Reg:")
         .run_pass(Ssa::as_slice_optimization, "After `as_slice` optimization")

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -255,7 +255,7 @@ impl AcirContext {
     }
 
     /// Converts an [`AcirVar`] to a [`Witness`]
-    fn var_to_witness(&mut self, var: AcirVar) -> Result<Witness, InternalError> {
+    pub(crate) fn var_to_witness(&mut self, var: AcirVar) -> Result<Witness, InternalError> {
         let expression = self.var_to_expression(var)?;
         let witness = if let Some(constant) = expression.to_const() {
             // Check if a witness has been assigned this value already, if so reuse it.
@@ -979,6 +979,7 @@ impl AcirContext {
         let max_power_of_two = self.add_constant(
             FieldElement::from(2_i128).pow(&FieldElement::from(bit_size as i128 - 1)),
         );
+        let zero = self.add_constant(FieldElement::zero());
         let one = self.add_constant(FieldElement::one());
 
         // Get the sign bit of rhs by computing rhs / max_power_of_two
@@ -998,10 +999,19 @@ impl AcirContext {
         // Unsigned to signed: derive q and r from q1,r1 and the signs of lhs and rhs
         // Quotient sign is lhs sign * rhs sign, whose resulting sign bit is the XOR of the sign bits
         let q_sign = self.xor_var(lhs_leading, rhs_leading, AcirType::unsigned(1))?;
-
         let quotient = self.two_complement(q1, q_sign, bit_size)?;
         let remainder = self.two_complement(r1, lhs_leading, bit_size)?;
 
+        // Issue #5129 - When q1 is zero and quotient sign is -1, we compute -0=2^{bit_size},
+        // which is not valid because we do not wrap integer operations
+        // Similar case can happen with the remainder.
+        let q_is_0 = self.eq_var(q1, zero)?;
+        let q_is_not_0 = self.not_var(q_is_0, AcirType::unsigned(1))?;
+        let quotient = self.mul_var(quotient, q_is_not_0)?;
+        let r_is_0 = self.eq_var(r1, zero)?;
+        let r_is_not_0 = self.not_var(r_is_0, AcirType::unsigned(1))?;
+        let remainder = self.mul_var(remainder, r_is_not_0)?;
+
         Ok((quotient, remainder))
     }
 
@@ -1017,15 +1027,6 @@ impl AcirContext {
         Ok(remainder)
     }
 
-    /// Converts the `AcirVar` to a `Witness` if it hasn't been already, and appends it to the
-    /// `GeneratedAcir`'s return witnesses.
-    pub(crate) fn return_var(&mut self, acir_var: AcirVar) -> Result<(), InternalError> {
-        let return_var = self.get_or_create_witness_var(acir_var)?;
-        let witness = self.var_to_witness(return_var)?;
-        self.acir_ir.push_return_witness(witness);
-        Ok(())
-    }
-
     /// Constrains the `AcirVar` variable to be of type `NumericType`.
     pub(crate) fn range_constrain_var(
         &mut self,
@@ -1528,9 +1529,11 @@ impl AcirContext {
     pub(crate) fn finish(
         mut self,
         inputs: Vec<Witness>,
+        return_values: Vec<Witness>,
         warnings: Vec<SsaReport>,
     ) -> GeneratedAcir {
         self.acir_ir.input_witnesses = inputs;
+        self.acir_ir.return_witnesses = return_values;
         self.acir_ir.warnings = warnings;
         self.acir_ir
     }

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs

@@ -45,9 +45,6 @@ pub(crate) struct GeneratedAcir {
     opcodes: Vec<AcirOpcode<FieldElement>>,
 
     /// All witness indices that comprise the final return value of the program
-    ///
-    /// Note: This may contain repeated indices, which is necessary for later mapping into the
-    /// abi's return type.
     pub(crate) return_witnesses: Vec<Witness>,
 
     /// All witness indices which are inputs to the main function
@@ -164,11 +161,6 @@ impl GeneratedAcir {
 
         fresh_witness
     }
-
-    /// Adds a witness index to the program's return witnesses.
-    pub(crate) fn push_return_witness(&mut self, witness: Witness) {
-        self.return_witnesses.push(witness);
-    }
 }
 
 impl GeneratedAcir {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -36,11 +36,8 @@ use acvm::acir::circuit::brillig::BrilligBytecode;
 use acvm::acir::circuit::{AssertionPayload, ErrorSelector, OpcodeLocation};
 use acvm::acir::native_types::Witness;
 use acvm::acir::BlackBoxFunc;
-use acvm::{
-    acir::AcirField,
-    acir::{circuit::opcodes::BlockId, native_types::Expression},
-    FieldElement,
-};
+use acvm::{acir::circuit::opcodes::BlockId, acir::AcirField, FieldElement};
+
 use fxhash::FxHashMap as HashMap;
 use im::Vector;
 use iter_extended::{try_vecmap, vecmap};
@@ -330,38 +327,10 @@ impl Ssa {
             bytecode: brillig.byte_code,
         });
 
-        let runtime_types = self.functions.values().map(|function| function.runtime());
-        for (acir, runtime_type) in acirs.iter_mut().zip(runtime_types) {
-            if matches!(runtime_type, RuntimeType::Acir(_)) {
-                generate_distinct_return_witnesses(acir);
-            }
-        }
-
         Ok((acirs, brillig, self.error_selector_to_type))
     }
 }
 
-fn generate_distinct_return_witnesses(acir: &mut GeneratedAcir) {
-    // Create a witness for each return witness we have to guarantee that the return witnesses match the standard
-    // layout for serializing those types as if they were being passed as inputs.
-    //
-    // This is required for recursion as otherwise in situations where we cannot make use of the program's ABI
-    // (e.g. for `std::verify_proof` or the solidity verifier), we need extra knowledge about the program we're
-    // working with rather than following the standard ABI encoding rules.
-    //
-    // TODO: We're being conservative here by generating a new witness for every expression.
-    // This means that we're likely to get a number of constraints which are just renumbering witnesses.
-    // This can be tackled by:
-    // - Tracking the last assigned public input witness and only renumbering a witness if it is below this value.
-    // - Modifying existing constraints to rearrange their outputs so they are suitable
-    //   - See: https://github.com/noir-lang/noir/pull/4467
-    let distinct_return_witness = vecmap(acir.return_witnesses.clone(), |return_witness| {
-        acir.create_witness_for_expression(&Expression::from(return_witness))
-    });
-
-    acir.return_witnesses = distinct_return_witness;
-}
-
 impl<'a> Context<'a> {
     fn new(shared_context: &'a mut SharedContext) -> Context<'a> {
         let mut acir_context = AcirContext::default();
@@ -422,15 +391,45 @@ impl<'a> Context<'a> {
         let dfg = &main_func.dfg;
         let entry_block = &dfg[main_func.entry_block()];
         let input_witness = self.convert_ssa_block_params(entry_block.parameters(), dfg)?;
+        let num_return_witnesses =
+            self.get_num_return_witnesses(entry_block.unwrap_terminator(), dfg);
+
+        // Create a witness for each return witness we have to guarantee that the return witnesses match the standard
+        // layout for serializing those types as if they were being passed as inputs.
+        //
+        // This is required for recursion as otherwise in situations where we cannot make use of the program's ABI
+        // (e.g. for `std::verify_proof` or the solidity verifier), we need extra knowledge about the program we're
+        // working with rather than following the standard ABI encoding rules.
+        //
+        // We allocate these witnesses now before performing ACIR gen for the rest of the program as the location of
+        // the function's return values can then be determined through knowledge of its ABI alone.
+        let return_witness_vars =
+            vecmap(0..num_return_witnesses, |_| self.acir_context.add_variable());
+
+        let return_witnesses = vecmap(&return_witness_vars, |return_var| {
+            let expr = self.acir_context.var_to_expression(*return_var).unwrap();
+            expr.to_witness().expect("return vars should be witnesses")
+        });
 
         self.data_bus = dfg.data_bus.to_owned();
         let mut warnings = Vec::new();
         for instruction_id in entry_block.instructions() {
             warnings.extend(self.convert_ssa_instruction(*instruction_id, dfg, ssa, brillig)?);
         }
 
-        warnings.extend(self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?);
-        Ok(self.acir_context.finish(input_witness, warnings))
+        let (return_vars, return_warnings) =
+            self.convert_ssa_return(entry_block.unwrap_terminator(), dfg)?;
+
+        // TODO: This is a naive method of assigning the return values to their witnesses as
+        // we're likely to get a number of constraints which are asserting one witness to be equal to another.
+        //
+        // We should search through the program and relabel these witnesses so we can remove this constraint.
+        for (witness_var, return_var) in return_witness_vars.iter().zip(return_vars) {
+            self.acir_context.assert_eq_var(*witness_var, return_var, None)?;
+        }
+
+        warnings.extend(return_warnings);
+        Ok(self.acir_context.finish(input_witness, return_witnesses, warnings))
     }
 
     fn convert_brillig_main(
@@ -468,17 +467,13 @@ impl<'a> Context<'a> {
         )?;
         self.shared_context.insert_generated_brillig(main_func.id(), arguments, 0, code);
 
-        let output_vars: Vec<_> = output_values
+        let return_witnesses: Vec<Witness> = output_values
             .iter()
             .flat_map(|value| value.clone().flatten())
-            .map(|value| value.0)
-            .collect();
-
-        for acir_var in output_vars {
-            self.acir_context.return_var(acir_var)?;
-        }
+            .map(|(value, _)| self.acir_context.var_to_witness(value))
+            .collect::<Result<_, _>>()?;
 
-        let generated_acir = self.acir_context.finish(witness_inputs, Vec::new());
+        let generated_acir = self.acir_context.finish(witness_inputs, return_witnesses, Vec::new());
 
         assert_eq!(
             generated_acir.opcodes().len(),
@@ -1724,12 +1719,39 @@ impl<'a> Context<'a> {
         self.define_result(dfg, instruction, AcirValue::Var(result, typ));
     }
 
+    /// Converts an SSA terminator's return values into their ACIR representations
+    fn get_num_return_witnesses(
+        &mut self,
+        terminator: &TerminatorInstruction,
+        dfg: &DataFlowGraph,
+    ) -> usize {
+        let return_values = match terminator {
+            TerminatorInstruction::Return { return_values, .. } => return_values,
+            // TODO(https://github.com/noir-lang/noir/issues/4616): Enable recursion on foldable/non-inlined ACIR functions
+            _ => unreachable!("ICE: Program must have a singular return"),
+        };
+
+        return_values.iter().fold(0, |acc, value_id| {
+            let is_databus = self
+                .data_bus
+                .return_data
+                .map_or(false, |return_databus| dfg[*value_id] == dfg[return_databus]);
+
+            if is_databus {
+                // We do not return value for the data bus.
+                acc
+            } else {
+                acc + dfg.type_of_value(*value_id).flattened_size()
+            }
+        })
+    }
+
     /// Converts an SSA terminator's return values into their ACIR representations
     fn convert_ssa_return(
         &mut self,
         terminator: &TerminatorInstruction,
         dfg: &DataFlowGraph,
-    ) -> Result<Vec<SsaReport>, RuntimeError> {
+    ) -> Result<(Vec<AcirVar>, Vec<SsaReport>), RuntimeError> {
         let (return_values, call_stack) = match terminator {
             TerminatorInstruction::Return { return_values, call_stack } => {
                 (return_values, call_stack.clone())
@@ -1739,6 +1761,7 @@ impl<'a> Context<'a> {
         };
 
         let mut has_constant_return = false;
+        let mut return_vars: Vec<AcirVar> = Vec::new();
         for value_id in return_values {
             let is_databus = self
                 .data_bus
@@ -1759,7 +1782,7 @@ impl<'a> Context<'a> {
                         dfg,
                     )?;
                 } else {
-                    self.acir_context.return_var(acir_var)?;
+                    return_vars.push(acir_var);
                 }
             }
         }
@@ -1770,7 +1793,7 @@ impl<'a> Context<'a> {
             Vec::new()
         };
 
-        Ok(warnings)
+        Ok((return_vars, warnings))
     }
 
     /// Gets the cached `AcirVar` that was converted from the corresponding `ValueId`. If it does
@@ -3079,8 +3102,8 @@ mod test {
         check_call_opcode(
             &func_with_nested_call_opcodes[1],
             2,
-            vec![Witness(2), Witness(1)],
-            vec![Witness(3)],
+            vec![Witness(3), Witness(1)],
+            vec![Witness(4)],
         );
     }
 
@@ -3100,13 +3123,13 @@ mod test {
                 for (expected_input, input) in expected_inputs.iter().zip(inputs) {
                     assert_eq!(
                         expected_input, input,
-                        "Expected witness {expected_input:?} but got {input:?}"
+                        "Expected input witness {expected_input:?} but got {input:?}"
                     );
                 }
                 for (expected_output, output) in expected_outputs.iter().zip(outputs) {
                     assert_eq!(
                         expected_output, output,
-                        "Expected witness {expected_output:?} but got {output:?}"
+                        "Expected output witness {expected_output:?} but got {output:?}"
                     );
                 }
             }

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/dfg.rs

@@ -22,7 +22,7 @@ use noirc_errors::Location;
 /// its blocks, instructions, and values. This struct is largely responsible for
 /// owning most data in a function and handing out Ids to this data that can be
 /// shared without worrying about ownership.
-#[derive(Debug, Default)]
+#[derive(Debug, Default, Clone)]
 pub(crate) struct DataFlowGraph {
     /// All of the instructions in a function
     instructions: DenseMap<Instruction>,

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/function.rs

@@ -64,6 +64,13 @@ impl Function {
         Self { name, id, entry_block, dfg, runtime: RuntimeType::Acir(InlineType::default()) }
     }
 
+    /// Creates a new function as a clone of the one passed in with the passed in id.
+    pub(crate) fn clone_with_id(id: FunctionId, another: &Function) -> Self {
+        let dfg = another.dfg.clone();
+        let entry_block = another.entry_block;
+        Self { name: another.name.clone(), id, entry_block, dfg, runtime: another.runtime }
+    }
+
     /// The name of the function.
     /// Used exclusively for debugging purposes.
     pub(crate) fn name(&self) -> &str {