Merge branch 'master' into michaeljklein/noir-cfg

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

@@ -52,7 +52,7 @@ jobs:
           tool: [email protected]
 
       - name: Build and archive tests
-        run: cargo nextest archive --workspace --release --archive-file nextest-archive.tar.zst
+        run: cargo nextest archive --workspace --archive-file nextest-archive.tar.zst
 
       - name: Upload archive to workflow
         uses: actions/upload-artifact@v4

.github/workflows/test-rust-workspace.yml

@@ -29,7 +29,7 @@ jobs:
 
       - uses: Swatinem/rust-cache@v2
         with:
-          key: x86_64-unknown-linux-gnu
+          key: x86_64-unknown-linux-gnu-debug
           cache-on-failure: true
           save-if: ${{ github.event_name != 'merge_group' }}
 
@@ -39,7 +39,7 @@ jobs:
           tool: [email protected]
 
       - name: Build and archive tests
-        run: cargo nextest archive --workspace --release --archive-file nextest-archive.tar.zst
+        run: cargo nextest archive --workspace --archive-file nextest-archive.tar.zst
 
       - name: Upload archive to workflow
         uses: actions/upload-artifact@v4

compiler/fm/src/file_map.rs

@@ -19,6 +19,10 @@ impl PathString {
     pub fn from_path(p: PathBuf) -> Self {
         PathString(p)
     }
+
+    pub fn into_path_buf(self) -> PathBuf {
+        self.0
+    }
 }
 impl From<PathBuf> for PathString {
     fn from(pb: PathBuf) -> PathString {
@@ -82,7 +86,7 @@ impl FileMap {
     }
 
     pub fn get_name(&self, file_id: FileId) -> Result<PathString, Error> {
-        let name = self.files.get(file_id.as_usize())?.name().clone();
+        let name = self.get_absolute_name(file_id)?;
 
         // See if we can make the file name a bit shorter/easier to read if it starts with the current directory
         if let Some(current_dir) = &self.current_dir {
@@ -93,6 +97,11 @@ impl FileMap {
 
         Ok(name)
     }
+
+    pub fn get_absolute_name(&self, file_id: FileId) -> Result<PathString, Error> {
+        let name = self.files.get(file_id.as_usize())?.name().clone();
+        Ok(name)
+    }
 }
 impl Default for FileMap {
     fn default() -> Self {

compiler/noirc_driver/src/abi_gen.rs

@@ -110,7 +110,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
             AbiType::String { length: size }
         }
 
-        Type::Struct(def, args) => {
+        Type::DataType(def, args) => {
             let struct_type = def.borrow();
             let fields = struct_type.get_fields(args);
             let fields =

compiler/noirc_driver/src/lib.rs

@@ -549,7 +549,7 @@ fn compile_contract_inner(
                 let structs = structs
                     .into_iter()
                     .map(|struct_id| {
-                        let typ = context.def_interner.get_struct(struct_id);
+                        let typ = context.def_interner.get_type(struct_id);
                         let typ = typ.borrow();
                         let fields = vecmap(typ.get_fields(&[]), |(name, typ)| {
                             (name, abi_type_from_hir_type(context, &typ))

compiler/noirc_evaluator/src/acir/mod.rs

@@ -1334,7 +1334,7 @@ impl<'a> Context<'a> {
         typ: &Type,
     ) -> Result<AcirValue, RuntimeError> {
         match typ {
-            Type::Numeric(_) => self.array_get_value(&Type::field(), call_data_block, offset),
+            Type::Numeric(_) => self.array_get_value(typ, call_data_block, offset),
             Type::Array(arc, len) => {
                 let mut result = im::Vector::new();
                 for _i in 0..*len {

compiler/noirc_evaluator/src/brillig/brillig_gen.rs

@@ -63,6 +63,7 @@ pub(crate) fn gen_brillig_for(
         FunctionContext::return_values(func),
         func.id(),
         true,
+        brillig.globals_memory_size,
     );
     entry_point.name = func.name().to_string();
 

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

@@ -10,7 +10,7 @@ pub(crate) fn convert_ssa_globals(
     enable_debug_trace: bool,
     globals: &Function,
     used_globals: &HashSet<ValueId>,
-) -> (BrilligArtifact<FieldElement>, HashMap<ValueId, BrilligVariable>) {
+) -> (BrilligArtifact<FieldElement>, HashMap<ValueId, BrilligVariable>, usize) {
     let mut brillig_context = BrilligContext::new_for_global_init(enable_debug_trace);
     // The global space does not have globals itself
     let empty_globals = HashMap::default();
@@ -32,8 +32,10 @@ pub(crate) fn convert_ssa_globals(
 
     brillig_block.compile_globals(&globals.dfg, used_globals);
 
+    let globals_size = brillig_block.brillig_context.global_space_size();
+
     brillig_context.return_instruction();
 
     let artifact = brillig_context.artifact();
-    (artifact, function_context.ssa_value_allocations)
+    (artifact, function_context.ssa_value_allocations, globals_size)
 }

compiler/noirc_evaluator/src/brillig/brillig_ir.rs

@@ -95,6 +95,8 @@ pub(crate) struct BrilligContext<F, Registers> {
     /// Whether this context can call procedures or not.
     /// This is used to prevent a procedure from calling another procedure.
     can_call_procedures: bool,
+
+    globals_memory_size: Option<usize>,
 }
 
 /// Regular brillig context to codegen user defined functions
@@ -108,6 +110,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
             next_section: 1,
             debug_show: DebugShow::new(enable_debug_trace),
             can_call_procedures: true,
+            globals_memory_size: None,
         }
     }
 }
@@ -211,6 +214,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, ScratchSpace> {
             next_section: 1,
             debug_show: DebugShow::new(enable_debug_trace),
             can_call_procedures: false,
+            globals_memory_size: None,
         }
     }
 }
@@ -226,8 +230,14 @@ impl<F: AcirField + DebugToString> BrilligContext<F, GlobalSpace> {
             next_section: 1,
             debug_show: DebugShow::new(enable_debug_trace),
             can_call_procedures: false,
+            globals_memory_size: None,
         }
     }
+
+    pub(crate) fn global_space_size(&self) -> usize {
+        // `GlobalSpace::start()` is inclusive so we must add one to get the accurate total global memory size
+        (self.registers.max_memory_address() + 1) - GlobalSpace::start()
+    }
 }
 
 impl<F: AcirField + DebugToString, Registers: RegisterAllocator> BrilligContext<F, Registers> {
@@ -321,6 +331,7 @@ pub(crate) mod tests {
             returns,
             FunctionId::test_new(0),
             false,
+            0,
         );
         entry_point_artifact.link_with(&artifact);
         while let Some(unresolved_fn_label) = entry_point_artifact.first_unresolved_function_call()

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

@@ -15,7 +15,6 @@ use acvm::acir::{
 pub(crate) const MAX_STACK_SIZE: usize = 16 * MAX_STACK_FRAME_SIZE;
 pub(crate) const MAX_STACK_FRAME_SIZE: usize = 2048;
 pub(crate) const MAX_SCRATCH_SPACE: usize = 64;
-pub(crate) const MAX_GLOBAL_SPACE: usize = 16384;
 
 impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
     /// Creates an entry point artifact that will jump to the function label provided.
@@ -24,9 +23,12 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
         return_parameters: Vec<BrilligParameter>,
         target_function: FunctionId,
         globals_init: bool,
+        globals_memory_size: usize,
     ) -> BrilligArtifact<F> {
         let mut context = BrilligContext::new(false);
 
+        context.globals_memory_size = Some(globals_memory_size);
+
         context.codegen_entry_point(&arguments, &return_parameters);
 
         if globals_init {
@@ -39,12 +41,15 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
         context.artifact()
     }
 
-    fn calldata_start_offset() -> usize {
-        ReservedRegisters::len() + MAX_STACK_SIZE + MAX_SCRATCH_SPACE + MAX_GLOBAL_SPACE
+    fn calldata_start_offset(&self) -> usize {
+        ReservedRegisters::len()
+            + MAX_STACK_SIZE
+            + MAX_SCRATCH_SPACE
+            + self.globals_memory_size.expect("The memory size of globals should be set")
     }
 
-    fn return_data_start_offset(calldata_size: usize) -> usize {
-        Self::calldata_start_offset() + calldata_size
+    fn return_data_start_offset(&self, calldata_size: usize) -> usize {
+        self.calldata_start_offset() + calldata_size
     }
 
     /// Adds the instructions needed to handle entry point parameters
@@ -70,7 +75,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
         // Set initial value of free memory pointer: calldata_start_offset + calldata_size + return_data_size
         self.const_instruction(
             SingleAddrVariable::new_usize(ReservedRegisters::free_memory_pointer()),
-            (Self::calldata_start_offset() + calldata_size + return_data_size).into(),
+            (self.calldata_start_offset() + calldata_size + return_data_size).into(),
         );
 
         // Set initial value of stack pointer: ReservedRegisters.len()
@@ -82,7 +87,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
         // Copy calldata
         self.copy_and_cast_calldata(arguments);
 
-        let mut current_calldata_pointer = Self::calldata_start_offset();
+        let mut current_calldata_pointer = self.calldata_start_offset();
 
         // Initialize the variables with the calldata
         for (argument_variable, argument) in argument_variables.iter_mut().zip(arguments) {
@@ -158,7 +163,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
     fn copy_and_cast_calldata(&mut self, arguments: &[BrilligParameter]) {
         let calldata_size = Self::flattened_tuple_size(arguments);
         self.calldata_copy_instruction(
-            MemoryAddress::direct(Self::calldata_start_offset()),
+            MemoryAddress::direct(self.calldata_start_offset()),
             calldata_size,
             0,
         );
@@ -178,11 +183,11 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
             if bit_size < F::max_num_bits() {
                 self.cast_instruction(
                     SingleAddrVariable::new(
-                        MemoryAddress::direct(Self::calldata_start_offset() + i),
+                        MemoryAddress::direct(self.calldata_start_offset() + i),
                         bit_size,
                     ),
                     SingleAddrVariable::new_field(MemoryAddress::direct(
-                        Self::calldata_start_offset() + i,
+                        self.calldata_start_offset() + i,
                     )),
                 );
             }
@@ -336,7 +341,7 @@ impl<F: AcirField + DebugToString> BrilligContext<F, Stack> {
         let return_data_size = Self::flattened_tuple_size(return_parameters);
 
         // Return data has a reserved space after calldata
-        let return_data_offset = Self::return_data_start_offset(calldata_size);
+        let return_data_offset = self.return_data_start_offset(calldata_size);
         let mut return_data_index = return_data_offset;
 
         for (return_param, returned_variable) in return_parameters.iter().zip(&returned_variables) {

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

@@ -7,7 +7,7 @@ use crate::brillig::brillig_ir::entry_point::MAX_STACK_SIZE;
 
 use super::{
     brillig_variable::SingleAddrVariable,
-    entry_point::{MAX_GLOBAL_SPACE, MAX_SCRATCH_SPACE, MAX_STACK_FRAME_SIZE},
+    entry_point::{MAX_SCRATCH_SPACE, MAX_STACK_FRAME_SIZE},
     BrilligContext, ReservedRegisters,
 };
 
@@ -152,16 +152,32 @@ impl RegisterAllocator for ScratchSpace {
 /// and is read-only.
 pub(crate) struct GlobalSpace {
     storage: DeallocationListAllocator,
+    max_memory_address: usize,
 }
 
 impl GlobalSpace {
-    pub(crate) fn new() -> Self {
-        Self { storage: DeallocationListAllocator::new(Self::start()) }
+    pub(super) fn new() -> Self {
+        Self {
+            storage: DeallocationListAllocator::new(Self::start()),
+            max_memory_address: Self::start(),
+        }
     }
 
     fn is_within_bounds(register: MemoryAddress) -> bool {
         let index = register.unwrap_direct();
-        index >= Self::start() && index < Self::end()
+        index >= Self::start()
+    }
+
+    fn update_max_address(&mut self, register: MemoryAddress) {
+        let index = register.unwrap_direct();
+        assert!(index >= Self::start(), "Global space malformed");
+        if index > self.max_memory_address {
+            self.max_memory_address = index;
+        }
+    }
+
+    pub(super) fn max_memory_address(&self) -> usize {
+        self.max_memory_address
     }
 }
 
@@ -171,12 +187,12 @@ impl RegisterAllocator for GlobalSpace {
     }
 
     fn end() -> usize {
-        Self::start() + MAX_GLOBAL_SPACE
+        unreachable!("The global space is set by the program");
     }
 
     fn allocate_register(&mut self) -> MemoryAddress {
         let allocated = MemoryAddress::direct(self.storage.allocate_register());
-        assert!(Self::is_within_bounds(allocated), "Global space too deep");
+        self.update_max_address(allocated);
         allocated
     }
 
@@ -185,7 +201,7 @@ impl RegisterAllocator for GlobalSpace {
     }
 
     fn ensure_register_is_allocated(&mut self, register: MemoryAddress) {
-        assert!(Self::is_within_bounds(register), "Register out of global space bounds");
+        self.update_max_address(register);
         self.storage.ensure_register_is_allocated(register.unwrap_direct());
     }
 
@@ -199,6 +215,7 @@ impl RegisterAllocator for GlobalSpace {
                 Self::start(),
                 vecmap(preallocated_registers, |r| r.unwrap_direct()),
             ),
+            max_memory_address: Self::start(),
         }
     }
 

compiler/noirc_evaluator/src/brillig/mod.rs

@@ -32,6 +32,7 @@ pub struct Brillig {
     /// Maps SSA function labels to their brillig artifact
     ssa_function_to_brillig: HashMap<FunctionId, BrilligArtifact<FieldElement>>,
     globals: BrilligArtifact<FieldElement>,
+    globals_memory_size: usize,
 }
 
 impl Brillig {
@@ -84,9 +85,10 @@ impl Ssa {
 
         let mut brillig = Brillig::default();
 
-        let (artifact, brillig_globals) =
+        let (artifact, brillig_globals, globals_size) =
             convert_ssa_globals(enable_debug_trace, &self.globals, &self.used_global_values);
         brillig.globals = artifact;
+        brillig.globals_memory_size = globals_size;
 
         for brillig_function_id in brillig_reachable_function_ids {
             let func = &self.functions[&brillig_function_id];

compiler/noirc_evaluator/src/ssa/ir/dfg.rs

@@ -116,11 +116,14 @@ pub(crate) struct GlobalsGraph {
     /// All of the instructions in the global value space.
     /// These are expected to all be Instruction::MakeArray
     instructions: DenseMap<Instruction>,
+
+    #[serde(skip)]
+    constants: HashMap<(FieldElement, NumericType), ValueId>,
 }
 
 impl GlobalsGraph {
     pub(crate) fn from_dfg(dfg: DataFlowGraph) -> Self {
-        Self { values: dfg.values, instructions: dfg.instructions }
+        Self { values: dfg.values, instructions: dfg.instructions, constants: dfg.constants }
     }
 
     /// Iterate over every Value in this DFG in no particular order, including unused Values
@@ -386,6 +389,9 @@ impl DataFlowGraph {
         if let Some(id) = self.constants.get(&(constant, typ)) {
             return *id;
         }
+        if let Some(id) = self.globals.constants.get(&(constant, typ)) {
+            return *id;
+        }
         let id = self.values.insert(Value::NumericConstant { constant, typ });
         self.constants.insert((constant, typ), id);
         id

compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs

@@ -16,6 +16,7 @@ use crate::ssa::{
         function::Function,
         function_inserter::FunctionInserter,
         instruction::{binary::eval_constant_binary_op, BinaryOp, Instruction, InstructionId},
+        post_order::PostOrder,
         types::Type,
         value::ValueId,
     },
@@ -272,8 +273,10 @@ impl<'f> LoopInvariantContext<'f> {
     /// correct new value IDs based upon the `FunctionInserter` internal map.
     /// Leaving out this mapping could lead to instructions with values that do not exist.
     fn map_dependent_instructions(&mut self) {
-        let blocks = self.inserter.function.reachable_blocks();
-        for block in blocks {
+        let mut block_order = PostOrder::with_function(self.inserter.function).into_vec();
+        block_order.reverse();
+
+        for block in block_order {
             for instruction_id in self.inserter.function.dfg[block].take_instructions() {
                 self.inserter.push_instruction(instruction_id, block);
             }