feat!: return arrays instead of slices from to_be_radix functions (#…

…5851)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR experiments with removing the `limb_count` argument to the
`to_be_radix` methods on `Field` and replacing it with a numeric
generic. This allows us to return an array rather than a slice which
should allow better codegen.

## Additional Context



## Documentation\*

Check one:
- [ ] No documentation needed.
- [x] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

.github/workflows/test-js-packages.yml

@@ -509,59 +509,6 @@ jobs:
         working-directory: ./examples/codegen_verifier
         run: ./test.sh
 
-  external-repo-checks:
-    needs: [build-nargo]
-    runs-on: ubuntu-22.04
-    timeout-minutes: 30
-    strategy:
-      fail-fast: false
-      matrix:
-        project:
-        #  - { repo: AztecProtocol/aztec-nr, path: ./ }
-        #  - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts }
-         # Disabled as aztec-packages requires a setup-step in order to generate a `Nargo.toml`
-         #- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits }
-         - { repo: zac-williamson/noir-edwards, path: ./, ref: 0016ce82cd58b6ebb0c43c271725590bcff4e755  }
-        # TODO: Enable these once they're passing against master again.
-        #  - { repo: zac-williamson/noir-bignum, path: ./, ref: 030c2acce1e6b97c44a3bbbf3429ed96f20d72d3  }
-        #  - { repo: vlayer-xyz/monorepo, path: ./, ref: ee46af88c025863872234eb05d890e1e447907cb }
-        #  - { repo: hashcloak/noir-bigint, path: ./, ref: 940ddba3a5201b508e7b37a2ef643551afcf5ed8 }
-
-    name: Check external repo - ${{ matrix.project.repo }}
-
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v4
-        with:
-          repository: ${{ matrix.project.repo }}
-          path: test-repo
-          ref: ${{ matrix.project.ref }}
-
-      - name: Download nargo binary
-        uses: actions/download-artifact@v4
-        with:
-          name: nargo
-          path: ./nargo
-
-      - name: Set nargo on PATH
-        run: |
-          nargo_binary="${{ github.workspace }}/nargo/nargo"
-          chmod +x $nargo_binary
-          echo "$(dirname $nargo_binary)" >> $GITHUB_PATH
-          export PATH="$PATH:$(dirname $nargo_binary)"
-          nargo -V
-      
-      - name: Remove requirements on compiler version
-        working-directory: ./test-repo
-        run: |
-          # Github actions seems to not expand "**" in globs by default.
-          shopt -s globstar
-          sed -i '/^compiler_version/d' ./**/Nargo.toml
-
-      - name: Run nargo check
-        working-directory: ./test-repo/${{ matrix.project.path }}
-        run: nargo check
-
   # This is a job which depends on all test jobs and reports the overall status.
   # This allows us to add/remove test jobs without having to update the required workflows.
   tests-end:

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

@@ -502,6 +502,8 @@ impl<'block> BrilligBlock<'block> {
                     );
                 }
                 Value::Intrinsic(Intrinsic::ToRadix(endianness)) => {
+                    let results = dfg.instruction_results(instruction_id);
+
                     let source = self.convert_ssa_single_addr_value(arguments[0], dfg);
 
                     let radix: u32 = dfg
@@ -512,88 +514,43 @@ impl<'block> BrilligBlock<'block> {
                         .try_into()
                         .expect("Radix should be u32");
 
-                    let limb_count: usize = dfg
-                        .get_numeric_constant(arguments[2])
-                        .expect("Limb count should be known")
-                        .try_to_u64()
-                        .expect("Limb count should fit in u64")
-                        .try_into()
-                        .expect("Limb count should fit in usize");
-
-                    let results = dfg.instruction_results(instruction_id);
-
-                    let target_len = self.variables.define_single_addr_variable(
-                        self.function_context,
-                        self.brillig_context,
-                        results[0],
-                        dfg,
-                    );
-
-                    let target_vector = self
+                    let target_array = self
                         .variables
                         .define_variable(
                             self.function_context,
                             self.brillig_context,
-                            results[1],
+                            results[0],
                             dfg,
                         )
-                        .extract_vector();
-
-                    // Update the user-facing slice length
-                    self.brillig_context
-                        .usize_const_instruction(target_len.address, limb_count.into());
+                        .extract_array();
 
                     self.brillig_context.codegen_to_radix(
                         source,
-                        target_vector,
+                        target_array,
                         radix,
-                        limb_count,
                         matches!(endianness, Endian::Big),
                         false,
                     );
                 }
                 Value::Intrinsic(Intrinsic::ToBits(endianness)) => {
-                    let source = self.convert_ssa_single_addr_value(arguments[0], dfg);
-                    let limb_count: usize = dfg
-                        .get_numeric_constant(arguments[1])
-                        .expect("Limb count should be known")
-                        .try_to_u64()
-                        .expect("Limb count should fit in u64")
-                        .try_into()
-                        .expect("Limb count should fit in usize");
-
                     let results = dfg.instruction_results(instruction_id);
 
-                    let target_len_variable = self.variables.define_variable(
-                        self.function_context,
-                        self.brillig_context,
-                        results[0],
-                        dfg,
-                    );
-                    let target_len = target_len_variable.extract_single_addr();
-
-                    let target_vector = match self.variables.define_variable(
-                        self.function_context,
-                        self.brillig_context,
-                        results[1],
-                        dfg,
-                    ) {
-                        BrilligVariable::BrilligArray(array) => {
-                            self.brillig_context.array_to_vector_instruction(&array)
-                        }
-                        BrilligVariable::BrilligVector(vector) => vector,
-                        BrilligVariable::SingleAddr(..) => unreachable!("ICE: ToBits on non-array"),
-                    };
+                    let source = self.convert_ssa_single_addr_value(arguments[0], dfg);
 
-                    // Update the user-facing slice length
-                    self.brillig_context
-                        .usize_const_instruction(target_len.address, limb_count.into());
+                    let target_array = self
+                        .variables
+                        .define_variable(
+                            self.function_context,
+                            self.brillig_context,
+                            results[0],
+                            dfg,
+                        )
+                        .extract_array();
 
                     self.brillig_context.codegen_to_radix(
                         source,
-                        target_vector,
+                        target_array,
                         2,
-                        limb_count,
                         matches!(endianness, Endian::Big),
                         true,
                     );

compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_intrinsic.rs

@@ -1,12 +1,12 @@
 use acvm::acir::{
-    brillig::{BlackBoxOp, HeapArray, IntegerBitSize},
+    brillig::{BlackBoxOp, IntegerBitSize},
     AcirField,
 };
 
 use crate::brillig::brillig_ir::BrilligBinaryOp;
 
 use super::{
-    brillig_variable::{BrilligVector, SingleAddrVariable},
+    brillig_variable::{BrilligArray, SingleAddrVariable},
     debug_show::DebugToString,
     registers::RegisterAllocator,
     BrilligContext,
@@ -67,27 +67,27 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
     pub(crate) fn codegen_to_radix(
         &mut self,
         source_field: SingleAddrVariable,
-        target_vector: BrilligVector,
+        target_array: BrilligArray,
         radix: u32,
-        limb_count: usize,
         big_endian: bool,
         output_bits: bool, // If true will generate bit limbs, if false will generate byte limbs
     ) {
         assert!(source_field.bit_size == F::max_num_bits());
 
-        self.usize_const_instruction(target_vector.size, limb_count.into());
-        self.usize_const_instruction(target_vector.rc, 1_usize.into());
-        self.codegen_allocate_array(target_vector.pointer, target_vector.size);
+        let size = SingleAddrVariable::new_usize(self.allocate_register());
+        self.usize_const_instruction(size.address, target_array.size.into());
+        self.usize_const_instruction(target_array.rc, 1_usize.into());
+        self.codegen_allocate_array(target_array.pointer, size.address);
 
         self.black_box_op_instruction(BlackBoxOp::ToRadix {
             input: source_field.address,
             radix,
-            output: HeapArray { pointer: target_vector.pointer, size: limb_count },
+            output: target_array.to_heap_array(),
             output_bits,
         });
 
         if big_endian {
-            self.codegen_array_reverse(target_vector.pointer, target_vector.size);
+            self.codegen_array_reverse(target_array.pointer, size.address);
         }
     }
 }

compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_memory.rs

@@ -269,34 +269,28 @@ impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<
             self.call_array_reverse_procedure(pointer, size);
             return;
         }
+
         let iteration_count = self.allocate_register();
         self.codegen_usize_op(size, iteration_count, BrilligBinaryOp::UnsignedDiv, 2);
 
         let start_value_register = self.allocate_register();
-        let index_at_end_of_array = self.allocate_register();
         let end_value_register = self.allocate_register();
+        let index_at_end_of_array = self.allocate_register();
 
         self.mov_instruction(index_at_end_of_array, size);
 
         self.codegen_loop(iteration_count, |ctx, iterator_register| {
             // The index at the end of array is size - 1 - iterator
             ctx.codegen_usize_op_in_place(index_at_end_of_array, BrilligBinaryOp::Sub, 1);
+            let index_at_end_of_array_var = SingleAddrVariable::new_usize(index_at_end_of_array);
 
             // Load both values
             ctx.codegen_array_get(pointer, iterator_register, start_value_register);
-            ctx.codegen_array_get(
-                pointer,
-                SingleAddrVariable::new_usize(index_at_end_of_array),
-                end_value_register,
-            );
+            ctx.codegen_array_get(pointer, index_at_end_of_array_var, end_value_register);
 
             // Write both values
             ctx.codegen_array_set(pointer, iterator_register, end_value_register);
-            ctx.codegen_array_set(
-                pointer,
-                SingleAddrVariable::new_usize(index_at_end_of_array),
-                start_value_register,
-            );
+            ctx.codegen_array_set(pointer, index_at_end_of_array_var, start_value_register);
         });
 
         self.deallocate_register(iteration_count);

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -1527,9 +1527,9 @@ impl<F: AcirField> AcirContext<F> {
         endian: Endian,
         input_var: AcirVar,
         radix_var: AcirVar,
-        limb_count_var: AcirVar,
+        limb_count: u32,
         result_element_type: AcirType,
-    ) -> Result<Vec<AcirValue>, RuntimeError> {
+    ) -> Result<AcirValue, RuntimeError> {
         let radix = match self.vars[&radix_var].as_constant() {
             Some(radix) => radix.to_u128() as u32,
             None => {
@@ -1540,16 +1540,6 @@ impl<F: AcirField> AcirContext<F> {
             }
         };
 
-        let limb_count = match self.vars[&limb_count_var].as_constant() {
-            Some(limb_count) => limb_count.to_u128() as u32,
-            None => {
-                return Err(RuntimeError::InternalError(InternalError::NotAConstant {
-                    name: "limb_size".to_string(),
-                    call_stack: self.get_call_stack(),
-                }));
-            }
-        };
-
         let input_expr = self.var_to_expression(input_var)?;
 
         let bit_size = u32::BITS - (radix - 1).leading_zeros();
@@ -1566,22 +1556,19 @@ impl<F: AcirField> AcirContext<F> {
 
         // `Intrinsic::ToRadix` returns slices which are represented
         // by tuples with the structure (length, slice contents)
-        Ok(vec![
-            AcirValue::Var(self.add_constant(limb_vars.len()), AcirType::field()),
-            AcirValue::Array(limb_vars.into()),
-        ])
+        Ok(AcirValue::Array(limb_vars.into()))
     }
 
     /// Returns `AcirVar`s constrained to be the bit decomposition of the provided input
     pub(crate) fn bit_decompose(
         &mut self,
         endian: Endian,
         input_var: AcirVar,
-        limb_count_var: AcirVar,
+        limb_count: u32,
         result_element_type: AcirType,
-    ) -> Result<Vec<AcirValue>, RuntimeError> {
+    ) -> Result<AcirValue, RuntimeError> {
         let two_var = self.add_constant(2_u128);
-        self.radix_decompose(endian, input_var, two_var, limb_count_var, result_element_type)
+        self.radix_decompose(endian, input_var, two_var, limb_count, result_element_type)
     }
 
     /// Recursive helper to flatten a single AcirValue into the result vector.

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

@@ -2177,19 +2177,38 @@ impl<'a> Context<'a> {
             Intrinsic::ToRadix(endian) => {
                 let field = self.convert_value(arguments[0], dfg).into_var()?;
                 let radix = self.convert_value(arguments[1], dfg).into_var()?;
-                let limb_size = self.convert_value(arguments[2], dfg).into_var()?;
 
-                let result_type = Self::array_element_type(dfg, result_ids[1]);
+                let Type::Array(result_type, array_length) = dfg.type_of_value(result_ids[0])
+                else {
+                    unreachable!("ICE: ToRadix result must be an array");
+                };
 
-                self.acir_context.radix_decompose(endian, field, radix, limb_size, result_type)
+                self.acir_context
+                    .radix_decompose(
+                        endian,
+                        field,
+                        radix,
+                        array_length as u32,
+                        result_type[0].clone().into(),
+                    )
+                    .map(|array| vec![array])
             }
             Intrinsic::ToBits(endian) => {
                 let field = self.convert_value(arguments[0], dfg).into_var()?;
-                let bit_size = self.convert_value(arguments[1], dfg).into_var()?;
 
-                let result_type = Self::array_element_type(dfg, result_ids[1]);
+                let Type::Array(result_type, array_length) = dfg.type_of_value(result_ids[0])
+                else {
+                    unreachable!("ICE: ToRadix result must be an array");
+                };
 
-                self.acir_context.bit_decompose(endian, field, bit_size, result_type)
+                self.acir_context
+                    .bit_decompose(
+                        endian,
+                        field,
+                        array_length as u32,
+                        result_type[0].clone().into(),
+                    )
+                    .map(|array| vec![array])
             }
             Intrinsic::ArrayLen => {
                 let len = match self.convert_value(arguments[0], dfg) {

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

@@ -533,7 +533,7 @@ mod tests {
     fn insert_constant_call() {
         // `bits` should be an array of constants [1, 1, 1, 0...] of length 8:
         // let x = 7;
-        // let bits = x.to_le_bits(8);
+        // let bits: [u1; 8] = x.to_le_bits();
         let func_id = Id::test_new(0);
         let mut builder = FunctionBuilder::new("func".into(), func_id);
         let one = builder.numeric_constant(FieldElement::one(), Type::bool());
@@ -546,13 +546,7 @@ mod tests {
         let call_results =
             builder.insert_call(to_bits_id, vec![input, length], result_types).into_owned();
 
-        let slice_len = match &builder.current_function.dfg[call_results[0]] {
-            Value::NumericConstant { constant, .. } => *constant,
-            _ => panic!(),
-        };
-        assert_eq!(slice_len, FieldElement::from(8_u128));
-
-        let slice = match &builder.current_function.dfg[call_results[1]] {
+        let slice = match &builder.current_function.dfg[call_results[0]] {
             Value::Array { array, .. } => array,
             _ => panic!(),
         };