Merge 2167743 into ce34fbd

.aztec-sync-commit

@@ -1 +1 @@
-249e50efafd306fa8cd9005972636adbddbca81e
+05cc59fd28b4d0ee89343106e538c0db0e70f52f

compiler/noirc_evaluator/src/brillig/brillig_gen.rs

@@ -4,6 +4,7 @@ pub(crate) mod brillig_block_variables;
 pub(crate) mod brillig_directive;
 pub(crate) mod brillig_fn;
 pub(crate) mod brillig_slice_ops;
+mod constant_allocation;
 mod variable_liveness;
 
 use acvm::FieldElement;

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

@@ -29,6 +29,7 @@ use std::sync::Arc;
 use super::brillig_black_box::convert_black_box_call;
 use super::brillig_block_variables::BlockVariables;
 use super::brillig_fn::FunctionContext;
+use super::constant_allocation::InstructionLocation;
 
 /// Generate the compilation artifacts for compiling a function into brillig bytecode.
 pub(crate) struct BrilligBlock<'block> {
@@ -117,6 +118,13 @@ impl<'block> BrilligBlock<'block> {
         terminator_instruction: &TerminatorInstruction,
         dfg: &DataFlowGraph,
     ) {
+        self.initialize_constants(
+            &self
+                .function_context
+                .constant_allocation
+                .allocated_at_location(self.block_id, InstructionLocation::Terminator),
+            dfg,
+        );
         match terminator_instruction {
             TerminatorInstruction::JmpIf {
                 condition,
@@ -244,6 +252,13 @@ impl<'block> BrilligBlock<'block> {
         let instruction = &dfg[instruction_id];
         self.brillig_context.set_call_stack(dfg.get_call_stack(instruction_id));
 
+        self.initialize_constants(
+            &self.function_context.constant_allocation.allocated_at_location(
+                self.block_id,
+                InstructionLocation::Instruction(instruction_id),
+            ),
+            dfg,
+        );
         match instruction {
             Instruction::Binary(binary) => {
                 let result_var = self.variables.define_single_addr_variable(
@@ -768,9 +783,6 @@ impl<'block> BrilligBlock<'block> {
             .brillig_context
             .codegen_pre_call_save_registers_prep_args(&argument_registers, &variables_to_save);
 
-        // We don't save and restore constants, so we dump them before a external call since the callee might use the registers where they are allocated.
-        self.variables.dump_constants();
-
         // Call instruction, which will interpret above registers 0..num args
         self.brillig_context.add_external_call_instruction(func_id);
 
@@ -1512,6 +1524,12 @@ impl<'block> BrilligBlock<'block> {
         }
     }
 
+    fn initialize_constants(&mut self, constants: &[ValueId], dfg: &DataFlowGraph) {
+        for &constant_id in constants {
+            self.convert_ssa_value(constant_id, dfg);
+        }
+    }
+
     /// Converts an SSA `ValueId` into a `RegisterOrMemory`. Initializes if necessary.
     fn convert_ssa_value(&mut self, value_id: ValueId, dfg: &DataFlowGraph) -> BrilligVariable {
         let value_id = dfg.resolve(value_id);
@@ -1527,23 +1545,31 @@ impl<'block> BrilligBlock<'block> {
             Value::NumericConstant { constant, .. } => {
                 // Constants might have been converted previously or not, so we get or create and
                 // (re)initialize the value inside.
-                if let Some(variable) = self.variables.get_constant(value_id, dfg) {
-                    variable
+                if self.variables.is_allocated(&value_id) {
+                    self.variables.get_allocation(self.function_context, value_id, dfg)
                 } else {
-                    let new_variable =
-                        self.variables.allocate_constant(self.brillig_context, value_id, dfg);
+                    let new_variable = self.variables.define_variable(
+                        self.function_context,
+                        self.brillig_context,
+                        value_id,
+                        dfg,
+                    );
 
                     self.brillig_context
                         .const_instruction(new_variable.extract_single_addr(), *constant);
                     new_variable
                 }
             }
             Value::Array { array, typ } => {
-                if let Some(variable) = self.variables.get_constant(value_id, dfg) {
-                    variable
+                if self.variables.is_allocated(&value_id) {
+                    self.variables.get_allocation(self.function_context, value_id, dfg)
                 } else {
-                    let new_variable =
-                        self.variables.allocate_constant(self.brillig_context, value_id, dfg);
+                    let new_variable = self.variables.define_variable(
+                        self.function_context,
+                        self.brillig_context,
+                        value_id,
+                        dfg,
+                    );
 
                     // Initialize the variable
                     let pointer = match new_variable {
@@ -1583,8 +1609,12 @@ impl<'block> BrilligBlock<'block> {
                 // around values representing function pointers, even though
                 // there is no interaction with the function possible given that
                 // value.
-                let new_variable =
-                    self.variables.allocate_constant(self.brillig_context, value_id, dfg);
+                let new_variable = self.variables.define_variable(
+                    self.function_context,
+                    self.brillig_context,
+                    value_id,
+                    dfg,
+                );
 
                 self.brillig_context.const_instruction(
                     new_variable.extract_single_addr(),
@@ -1732,18 +1762,10 @@ impl<'block> BrilligBlock<'block> {
         self.brillig_context.mov_instruction(write_pointer_register, pointer);
 
         for (element_idx, element_id) in data.iter().enumerate() {
-            if let Some((constant, typ)) = dfg.get_numeric_constant_with_type(*element_id) {
-                self.brillig_context.indirect_const_instruction(
-                    write_pointer_register,
-                    typ.bit_size(),
-                    constant,
-                );
-            } else {
-                let element_variable = self.convert_ssa_value(*element_id, dfg);
-                // Store the item in memory
-                self.brillig_context
-                    .codegen_store_variable_in_pointer(write_pointer_register, element_variable);
-            }
+            let element_variable = self.convert_ssa_value(*element_id, dfg);
+            // Store the item in memory
+            self.brillig_context
+                .codegen_store_variable_in_pointer(write_pointer_register, element_variable);
 
             if element_idx != data.len() - 1 {
                 // Increment the write_pointer_register

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

@@ -1,5 +1,5 @@
 use acvm::FieldElement;
-use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
+use fxhash::FxHashSet as HashSet;
 
 use crate::{
     brillig::brillig_ir::{
@@ -22,16 +22,15 @@ use super::brillig_fn::FunctionContext;
 #[derive(Debug, Default)]
 pub(crate) struct BlockVariables {
     available_variables: HashSet<ValueId>,
-    available_constants: HashMap<ValueId, BrilligVariable>,
 }
 
 impl BlockVariables {
     /// Creates a BlockVariables instance. It uses the variables that are live in to the block and the global available variables (block parameters)
     pub(crate) fn new(live_in: HashSet<ValueId>) -> Self {
-        BlockVariables { available_variables: live_in, ..Default::default() }
+        BlockVariables { available_variables: live_in }
     }
 
-    /// Returns all non-constant variables that have not been removed at this point.
+    /// Returns all variables that have not been removed at this point.
     pub(crate) fn get_available_variables(
         &self,
         function_context: &FunctionContext,
@@ -48,7 +47,7 @@ impl BlockVariables {
             .collect()
     }
 
-    /// For a given SSA non constant value id, define the variable and return the corresponding cached allocation.
+    /// For a given SSA value id, define the variable and return the corresponding cached allocation.
     pub(crate) fn define_variable(
         &mut self,
         function_context: &mut FunctionContext,
@@ -97,6 +96,11 @@ impl BlockVariables {
         });
     }
 
+    /// Checks if a variable is allocated.
+    pub(crate) fn is_allocated(&self, value_id: &ValueId) -> bool {
+        self.available_variables.contains(value_id)
+    }
+
     /// For a given SSA value id, return the corresponding cached allocation.
     pub(crate) fn get_allocation(
         &mut self,
@@ -105,48 +109,16 @@ impl BlockVariables {
         dfg: &DataFlowGraph,
     ) -> BrilligVariable {
         let value_id = dfg.resolve(value_id);
-        if let Some(constant) = self.available_constants.get(&value_id) {
-            *constant
-        } else {
-            assert!(
-                self.available_variables.contains(&value_id),
-                "ICE: ValueId {value_id:?} is not available"
-            );
-
-            *function_context
-                .ssa_value_allocations
-                .get(&value_id)
-                .unwrap_or_else(|| panic!("ICE: Value not found in cache {value_id}"))
-        }
-    }
-
-    /// Creates a constant. Constants are a special case in SSA, since they are "defined" every time they are used.
-    /// We keep constants block-local.
-    pub(crate) fn allocate_constant(
-        &mut self,
-        brillig_context: &mut BrilligContext<FieldElement, Stack>,
-        value_id: ValueId,
-        dfg: &DataFlowGraph,
-    ) -> BrilligVariable {
-        let value_id = dfg.resolve(value_id);
-        let constant = allocate_value(value_id, brillig_context, dfg);
-        self.available_constants.insert(value_id, constant);
-        constant
-    }
 
-    /// Gets a constant.
-    pub(crate) fn get_constant(
-        &mut self,
-        value_id: ValueId,
-        dfg: &DataFlowGraph,
-    ) -> Option<BrilligVariable> {
-        let value_id = dfg.resolve(value_id);
-        self.available_constants.get(&value_id).cloned()
-    }
+        assert!(
+            self.available_variables.contains(&value_id),
+            "ICE: ValueId {value_id:?} is not available"
+        );
 
-    /// Removes the allocations of all constants. Constants will need to be reallocated and reinitialized after this.
-    pub(crate) fn dump_constants(&mut self) {
-        self.available_constants.clear();
+        *function_context
+            .ssa_value_allocations
+            .get(&value_id)
+            .unwrap_or_else(|| panic!("ICE: Value not found in cache {value_id}"))
     }
 }
 

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

@@ -15,7 +15,7 @@ use crate::{
 };
 use fxhash::FxHashMap as HashMap;
 
-use super::variable_liveness::VariableLiveness;
+use super::{constant_allocation::ConstantAllocation, variable_liveness::VariableLiveness};
 
 pub(crate) struct FunctionContext {
     pub(crate) function_id: FunctionId,
@@ -25,6 +25,8 @@ pub(crate) struct FunctionContext {
     pub(crate) blocks: Vec<BasicBlockId>,
     /// Liveness information for each variable in the function.
     pub(crate) liveness: VariableLiveness,
+    /// Information on where to allocate constants
+    pub(crate) constant_allocation: ConstantAllocation,
 }
 
 impl FunctionContext {
@@ -36,11 +38,15 @@ impl FunctionContext {
         reverse_post_order.extend_from_slice(PostOrder::with_function(function).as_slice());
         reverse_post_order.reverse();
 
+        let constants = ConstantAllocation::from_function(function);
+        let liveness = VariableLiveness::from_function(function, &constants);
+
         Self {
             function_id: id,
             ssa_value_allocations: HashMap::default(),
             blocks: reverse_post_order,
-            liveness: VariableLiveness::from_function(function),
+            liveness,
+            constant_allocation: constants,
         }
     }