[1 changes] feat(ssa): Loop invariant code motion (noir-lang/noir#6563)

fix: remove `compiler_version` from new `Nargo.toml` (noir-lang/noir#6590)
feat: Avoid incrementing reference counts in some cases (noir-lang/noir#6568)
chore: fix typo in test name (noir-lang/noir#6589)
fix: consider prereleases to be compatible with pre-1.0.0 releases (noir-lang/noir#6580)
feat: try to inline brillig calls with all constant arguments  (noir-lang/noir#6548)
fix: correct type when simplifying `derive_pedersen_generators` (noir-lang/noir#6579)
feat: Sync from aztec-packages (noir-lang/noir#6576)

Author: AztecBot

.noir-sync-commit

@@ -1 +1 @@
-68c32b4ffd9b069fe4b119327dbf4018c17ab9d4
+7216f0829dcece948d3243471e6d57380522e997

noir/noir-repo/acvm-repo/acvm_js/build.sh

@@ -25,7 +25,7 @@ function run_if_available {
 require_command jq
 require_command cargo
 require_command wasm-bindgen
-#require_command wasm-opt
+require_command wasm-opt
 
 self_path=$(dirname "$(readlink -f "$0")")
 pname=$(cargo read-manifest | jq -r '.name')

noir/noir-repo/compiler/integration-tests/package.json

@@ -13,7 +13,7 @@
     "lint": "NODE_NO_WARNINGS=1 eslint . --ext .ts --ignore-path ./.eslintignore  --max-warnings 0"
   },
   "dependencies": {
-    "@aztec/bb.js": "portal:../../../../barretenberg/ts",
+    "@aztec/bb.js": "0.63.1",
     "@noir-lang/noir_js": "workspace:*",
     "@noir-lang/noir_wasm": "workspace:*",
     "@nomicfoundation/hardhat-chai-matchers": "^2.0.0",

noir/noir-repo/compiler/noirc_evaluator/src/acir/mod.rs

@@ -24,12 +24,10 @@ mod big_int;
 mod brillig_directive;
 mod generated_acir;
 
+use crate::brillig::brillig_gen::gen_brillig_for;
 use crate::brillig::{
     brillig_gen::brillig_fn::FunctionContext as BrilligFunctionContext,
-    brillig_ir::{
-        artifact::{BrilligParameter, GeneratedBrillig},
-        BrilligContext,
-    },
+    brillig_ir::artifact::{BrilligParameter, GeneratedBrillig},
     Brillig,
 };
 use crate::errors::{InternalError, InternalWarning, RuntimeError, SsaReport};
@@ -518,7 +516,7 @@ impl<'a> Context<'a> {
         let outputs: Vec<AcirType> =
             vecmap(main_func.returns(), |result_id| dfg.type_of_value(*result_id).into());
 
-        let code = self.gen_brillig_for(main_func, arguments.clone(), brillig)?;
+        let code = gen_brillig_for(main_func, arguments.clone(), brillig)?;
 
         // We specifically do not attempt execution of the brillig code being generated as this can result in it being
         // replaced with constraints on witnesses to the program outputs.
@@ -878,8 +876,7 @@ impl<'a> Context<'a> {
                                         None,
                                     )?
                                 } else {
-                                    let code =
-                                        self.gen_brillig_for(func, arguments.clone(), brillig)?;
+                                    let code = gen_brillig_for(func, arguments.clone(), brillig)?;
                                     let generated_pointer =
                                         self.shared_context.new_generated_pointer();
                                     let output_values = self.acir_context.brillig_call(
@@ -999,47 +996,6 @@ impl<'a> Context<'a> {
             .collect()
     }
 
-    fn gen_brillig_for(
-        &self,
-        func: &Function,
-        arguments: Vec<BrilligParameter>,
-        brillig: &Brillig,
-    ) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
-        // Create the entry point artifact
-        let mut entry_point = BrilligContext::new_entry_point_artifact(
-            arguments,
-            BrilligFunctionContext::return_values(func),
-            func.id(),
-        );
-        entry_point.name = func.name().to_string();
-
-        // Link the entry point with all dependencies
-        while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() {
-            let artifact = &brillig.find_by_label(unresolved_fn_label.clone());
-            let artifact = match artifact {
-                Some(artifact) => artifact,
-                None => {
-                    return Err(InternalError::General {
-                        message: format!("Cannot find linked fn {unresolved_fn_label}"),
-                        call_stack: CallStack::new(),
-                    })
-                }
-            };
-            entry_point.link_with(artifact);
-            // Insert the range of opcode locations occupied by a procedure
-            if let Some(procedure_id) = &artifact.procedure {
-                let num_opcodes = entry_point.byte_code.len();
-                let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
-                // We subtract one as to keep the range inclusive on both ends
-                entry_point
-                    .procedure_locations
-                    .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1));
-            }
-        }
-        // Generate the final bytecode
-        Ok(entry_point.finish())
-    }
-
     /// Handles an ArrayGet or ArraySet instruction.
     /// To set an index of the array (and create a new array in doing so), pass Some(value) for
     /// store_value. To just retrieve an index of the array, pass None for store_value.

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs

@@ -9,11 +9,17 @@ mod variable_liveness;
 use acvm::FieldElement;
 
 use self::{brillig_block::BrilligBlock, brillig_fn::FunctionContext};
-use super::brillig_ir::{
-    artifact::{BrilligArtifact, Label},
-    BrilligContext,
+use super::{
+    brillig_ir::{
+        artifact::{BrilligArtifact, BrilligParameter, GeneratedBrillig, Label},
+        BrilligContext,
+    },
+    Brillig,
+};
+use crate::{
+    errors::InternalError,
+    ssa::ir::{dfg::CallStack, function::Function},
 };
-use crate::ssa::ir::function::Function;
 
 /// Converting an SSA function into Brillig bytecode.
 pub(crate) fn convert_ssa_function(
@@ -36,3 +42,43 @@ pub(crate) fn convert_ssa_function(
     artifact.name = func.name().to_string();
     artifact
 }
+
+pub(crate) fn gen_brillig_for(
+    func: &Function,
+    arguments: Vec<BrilligParameter>,
+    brillig: &Brillig,
+) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
+    // Create the entry point artifact
+    let mut entry_point = BrilligContext::new_entry_point_artifact(
+        arguments,
+        FunctionContext::return_values(func),
+        func.id(),
+    );
+    entry_point.name = func.name().to_string();
+
+    // Link the entry point with all dependencies
+    while let Some(unresolved_fn_label) = entry_point.first_unresolved_function_call() {
+        let artifact = &brillig.find_by_label(unresolved_fn_label.clone());
+        let artifact = match artifact {
+            Some(artifact) => artifact,
+            None => {
+                return Err(InternalError::General {
+                    message: format!("Cannot find linked fn {unresolved_fn_label}"),
+                    call_stack: CallStack::new(),
+                })
+            }
+        };
+        entry_point.link_with(artifact);
+        // Insert the range of opcode locations occupied by a procedure
+        if let Some(procedure_id) = &artifact.procedure {
+            let num_opcodes = entry_point.byte_code.len();
+            let previous_num_opcodes = entry_point.byte_code.len() - artifact.byte_code.len();
+            // We subtract one as to keep the range inclusive on both ends
+            entry_point
+                .procedure_locations
+                .insert(procedure_id.clone(), (previous_num_opcodes, num_opcodes - 1));
+        }
+    }
+    // Generate the final bytecode
+    Ok(entry_point.finish())
+}

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

@@ -103,6 +103,7 @@ pub(crate) fn optimize_into_acir(
         Ssa::evaluate_static_assert_and_assert_constant,
         "After `static_assert` and `assert_constant`:",
     )?
+    .run_pass(Ssa::loop_invariant_code_motion, "After Loop Invariant Code Motion:")
     .try_run_pass(Ssa::unroll_loops_iteratively, "After Unrolling:")?
     .run_pass(Ssa::simplify_cfg, "After Simplifying (2nd):")
     .run_pass(Ssa::flatten_cfg, "After Flattening:")
@@ -140,6 +141,23 @@ pub(crate) fn optimize_into_acir(
         ssa.to_brillig(options.enable_brillig_logging)
     });
 
+    let ssa_gen_span = span!(Level::TRACE, "ssa_generation");
+    let ssa_gen_span_guard = ssa_gen_span.enter();
+
+    let ssa = SsaBuilder {
+        ssa,
+        print_ssa_passes: options.enable_ssa_logging,
+        print_codegen_timings: options.print_codegen_timings,
+    }
+    .run_pass(
+        |ssa| ssa.fold_constants_with_brillig(&brillig),
+        "After Constant Folding with Brillig:",
+    )
+    .run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
+    .finish();
+
+    drop(ssa_gen_span_guard);
+
     let artifacts = time("SSA to ACIR", options.print_codegen_timings, || {
         ssa.into_acir(&brillig, options.expression_width)
     })?;

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

@@ -25,7 +25,7 @@ pub(crate) struct FunctionInserter<'f> {
     ///
     /// This is optional since caching arrays relies on the inserter inserting strictly
     /// in control-flow order. Otherwise, if arrays later in the program are cached first,
-    /// they may be refered to by instructions earlier in the program.
+    /// they may be referred to by instructions earlier in the program.
     array_cache: Option<ArrayCache>,
 
     /// If this pass is loop unrolling, store the block before the loop to optionally

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

@@ -11,11 +11,12 @@ use fxhash::FxHasher64;
 use iter_extended::vecmap;
 use noirc_frontend::hir_def::types::Type as HirType;
 
-use crate::ssa::opt::flatten_cfg::value_merger::ValueMerger;
+use crate::ssa::{ir::function::RuntimeType, opt::flatten_cfg::value_merger::ValueMerger};
 
 use super::{
     basic_block::BasicBlockId,
     dfg::{CallStack, DataFlowGraph},
+    function::Function,
     map::Id,
     types::{NumericType, Type},
     value::{Value, ValueId},
@@ -269,15 +270,7 @@ pub(crate) enum Instruction {
     ///     else_value
     /// }
     /// ```
-    ///
-    /// Where we save the result of !then_condition so that we have the same
-    /// ValueId for it each time.
-    IfElse {
-        then_condition: ValueId,
-        then_value: ValueId,
-        else_condition: ValueId,
-        else_value: ValueId,
-    },
+    IfElse { then_condition: ValueId, then_value: ValueId, else_value: ValueId },
 
     /// Creates a new array or slice.
     ///
@@ -371,12 +364,12 @@ impl Instruction {
         }
     }
 
-    pub(crate) fn can_eliminate_if_unused(&self, dfg: &DataFlowGraph) -> bool {
+    pub(crate) fn can_eliminate_if_unused(&self, function: &Function) -> bool {
         use Instruction::*;
         match self {
             Binary(binary) => {
                 if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) {
-                    if let Some(rhs) = dfg.get_numeric_constant(binary.rhs) {
+                    if let Some(rhs) = function.dfg.get_numeric_constant(binary.rhs) {
                         rhs != FieldElement::zero()
                     } else {
                         false
@@ -395,15 +388,26 @@ impl Instruction {
             | ArraySet { .. }
             | MakeArray { .. } => true,
 
+            // Store instructions must be removed by DIE in acir code, any load
+            // instructions should already be unused by that point.
+            //
+            // Note that this check assumes that it is being performed after the flattening
+            // pass and after the last mem2reg pass. This is currently the case for the DIE
+            // pass where this check is done, but does mean that we cannot perform mem2reg
+            // after the DIE pass.
+            Store { .. } => {
+                matches!(function.runtime(), RuntimeType::Acir(_))
+                    && function.reachable_blocks().len() == 1
+            }
+
             Constrain(..)
-            | Store { .. }
             | EnableSideEffectsIf { .. }
             | IncrementRc { .. }
             | DecrementRc { .. }
             | RangeCheck { .. } => false,
 
             // Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.
-            Call { func, .. } => match dfg[*func] {
+            Call { func, .. } => match function.dfg[*func] {
                 // Explicitly allows removal of unused ec operations, even if they can fail
                 Value::Intrinsic(Intrinsic::BlackBox(BlackBoxFunc::MultiScalarMul))
                 | Value::Intrinsic(Intrinsic::BlackBox(BlackBoxFunc::EmbeddedCurveAdd)) => true,
@@ -524,14 +528,11 @@ impl Instruction {
                     assert_message: assert_message.clone(),
                 }
             }
-            Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
-                Instruction::IfElse {
-                    then_condition: f(*then_condition),
-                    then_value: f(*then_value),
-                    else_condition: f(*else_condition),
-                    else_value: f(*else_value),
-                }
-            }
+            Instruction::IfElse { then_condition, then_value, else_value } => Instruction::IfElse {
+                then_condition: f(*then_condition),
+                then_value: f(*then_value),
+                else_value: f(*else_value),
+            },
             Instruction::MakeArray { elements, typ } => Instruction::MakeArray {
                 elements: elements.iter().copied().map(f).collect(),
                 typ: typ.clone(),
@@ -590,10 +591,9 @@ impl Instruction {
             | Instruction::RangeCheck { value, .. } => {
                 f(*value);
             }
-            Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
+            Instruction::IfElse { then_condition, then_value, else_value } => {
                 f(*then_condition);
                 f(*then_value);
-                f(*else_condition);
                 f(*else_value);
             }
             Instruction::MakeArray { elements, typ: _ } => {
@@ -756,7 +756,7 @@ impl Instruction {
                     None
                 }
             }
-            Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
+            Instruction::IfElse { then_condition, then_value, else_value } => {
                 let typ = dfg.type_of_value(*then_value);
 
                 if let Some(constant) = dfg.get_numeric_constant(*then_condition) {
@@ -775,13 +775,11 @@ impl Instruction {
 
                 if matches!(&typ, Type::Numeric(_)) {
                     let then_condition = *then_condition;
-                    let else_condition = *else_condition;
 
                     let result = ValueMerger::merge_numeric_values(
                         dfg,
                         block,
                         then_condition,
-                        else_condition,
                         then_value,
                         else_value,
                     );

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs

@@ -443,12 +443,8 @@ fn simplify_slice_push_back(
     let mut value_merger =
         ValueMerger::new(dfg, block, &mut slice_sizes, unknown, None, call_stack);
 
-    let new_slice = value_merger.merge_values(
-        len_not_equals_capacity,
-        len_equals_capacity,
-        set_last_slice_value,
-        new_slice,
-    );
+    let new_slice =
+        value_merger.merge_values(len_not_equals_capacity, set_last_slice_value, new_slice);
 
     SimplifyResult::SimplifiedToMultiple(vec![new_slice_length, new_slice])
 }
@@ -810,7 +806,8 @@ fn simplify_derive_generators(
                 results.push(is_infinite);
             }
             let len = results.len();
-            let typ = Type::Array(vec![Type::field()].into(), len);
+            let typ =
+                Type::Array(vec![Type::field(), Type::field(), Type::unsigned(1)].into(), len / 3);
             let result = make_array(dfg, results.into(), typ, block, call_stack);
             SimplifyResult::SimplifiedTo(result)
         } else {
@@ -820,3 +817,34 @@ fn simplify_derive_generators(
         unreachable!("Unexpected number of arguments to derive_generators");
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::ssa::{opt::assert_normalized_ssa_equals, Ssa};
+
+    #[test]
+    fn simplify_derive_generators_has_correct_type() {
+        let src = "
+            brillig(inline) fn main f0 {
+              b0():
+                v0 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
+
+                // This call was previously incorrectly simplified to something that returned `[Field; 3]`
+                v2 = call derive_pedersen_generators(v0, u32 0) -> [(Field, Field, u1); 1]
+
+                return v2
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+
+        let expected = "
+            brillig(inline) fn main f0 {
+              b0():
+                v15 = make_array [u8 68, u8 69, u8 70, u8 65, u8 85, u8 76, u8 84, u8 95, u8 68, u8 79, u8 77, u8 65, u8 73, u8 78, u8 95, u8 83, u8 69, u8 80, u8 65, u8 82, u8 65, u8 84, u8 79, u8 82] : [u8; 24]
+                v19 = make_array [Field 3728882899078719075161482178784387565366481897740339799480980287259621149274, Field -9903063709032878667290627648209915537972247634463802596148419711785767431332, u1 0] : [(Field, Field, u1); 1]
+                return v19
+            }
+            ";
+        assert_normalized_ssa_equals(ssa, expected);
+    }
+}