feat: Sync from noir

.noir-sync-commit

@@ -1 +1 @@
-d4c68066ab35ce1c52510cf0c038fb627a0677c3
+a87c655c6c8c077c71e3372cc9181b7870348a3d

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

@@ -619,7 +619,7 @@ impl<'block> BrilligBlock<'block> {
                     destination_variable,
                 );
             }
-            Instruction::ArraySet { array, index, value, .. } => {
+            Instruction::ArraySet { array, index, value, mutable: _ } => {
                 let source_variable = self.convert_ssa_value(*array, dfg);
                 let index_register = self.convert_ssa_single_addr_value(*index, dfg);
                 let value_variable = self.convert_ssa_value(*value, dfg);
@@ -700,6 +700,9 @@ impl<'block> BrilligBlock<'block> {
             Instruction::EnableSideEffects { .. } => {
                 todo!("enable_side_effects not supported by brillig")
             }
+            Instruction::IfElse { .. } => {
+                unreachable!("IfElse instructions should not be possible in brillig")
+            }
         };
 
         let dead_variables = self

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

@@ -69,6 +69,7 @@ pub(crate) fn optimize_into_acir(
         // Run the inlining pass again to handle functions with `InlineType::NoPredicates`.
         // Before flattening is run, we treat functions marked with the `InlineType::NoPredicates` as an entry point.
         .run_pass(Ssa::inline_functions_with_no_predicates, "After Inlining:")
+        .run_pass(Ssa::remove_if_else, "After Remove IfElse:")
         .run_pass(Ssa::fold_constants, "After Constant Folding:")
         .run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffects removal:")
         .run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:")

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

@@ -705,6 +705,9 @@ impl<'a> Context<'a> {
                     assert_message.clone(),
                 )?;
             }
+            Instruction::IfElse { .. } => {
+                unreachable!("IfElse instruction remaining in acir-gen")
+            }
         }
 
         self.acir_context.set_call_stack(CallStack::new());
@@ -732,11 +735,10 @@ impl<'a> Context<'a> {
                                 assert!(!matches!(inline_type, InlineType::Inline), "ICE: Got an ACIR function named {} that should have already been inlined", func.name());
 
                                 let inputs = vecmap(arguments, |arg| self.convert_value(*arg, dfg));
-                                // TODO(https://github.com/noir-lang/noir/issues/4608): handle complex return types from ACIR functions
-                                let output_count =
-                                    result_ids.iter().fold(0usize, |sum, result_id| {
-                                        sum + dfg.try_get_array_length(*result_id).unwrap_or(1)
-                                    });
+                                let output_count = result_ids
+                                    .iter()
+                                    .map(|result_id| dfg.type_of_value(*result_id).flattened_size())
+                                    .sum();
 
                                 let acir_function_id = ssa
                                     .entry_point_to_generated_index
@@ -748,6 +750,7 @@ impl<'a> Context<'a> {
                                     output_count,
                                     self.current_side_effects_enabled_var,
                                 )?;
+
                                 let output_values =
                                     self.convert_vars_to_values(output_vars, dfg, result_ids);
 
@@ -1028,6 +1031,7 @@ impl<'a> Context<'a> {
                             });
                         }
                     };
+
                     if self.acir_context.is_constant_one(&self.current_side_effects_enabled_var) {
                         // Report the error if side effects are enabled.
                         if index >= array_size {

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

@@ -554,7 +554,10 @@ impl<'dfg> InsertInstructionResult<'dfg> {
         match self {
             InsertInstructionResult::SimplifiedTo(value) => *value,
             InsertInstructionResult::SimplifiedToMultiple(values) => values[0],
-            InsertInstructionResult::Results(_, results) => results[0],
+            InsertInstructionResult::Results(_, results) => {
+                assert_eq!(results.len(), 1);
+                results[0]
+            }
             InsertInstructionResult::InstructionRemoved => {
                 panic!("Instruction was removed, no results")
             }
@@ -583,6 +586,24 @@ impl<'dfg> InsertInstructionResult<'dfg> {
     }
 }
 
+impl<'dfg> std::ops::Index<usize> for InsertInstructionResult<'dfg> {
+    type Output = ValueId;
+
+    fn index(&self, index: usize) -> &Self::Output {
+        match self {
+            InsertInstructionResult::Results(_, results) => &results[index],
+            InsertInstructionResult::SimplifiedTo(result) => {
+                assert_eq!(index, 0);
+                result
+            }
+            InsertInstructionResult::SimplifiedToMultiple(results) => &results[index],
+            InsertInstructionResult::InstructionRemoved => {
+                panic!("Cannot index into InsertInstructionResult::InstructionRemoved")
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::DataFlowGraph;

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

@@ -11,6 +11,8 @@ use fxhash::FxHasher;
 use iter_extended::vecmap;
 use noirc_frontend::hir_def::types::Type as HirType;
 
+use crate::ssa::opt::flatten_cfg::value_merger::ValueMerger;
+
 use super::{
     basic_block::BasicBlockId,
     dfg::{CallStack, DataFlowGraph},
@@ -216,6 +218,14 @@ pub(crate) enum Instruction {
     /// implemented via reference counting. In ACIR code this is done with im::Vector and these
     /// DecrementRc instructions are ignored.
     DecrementRc { value: ValueId },
+
+    /// Merge two values returned from opposite branches of a conditional into one.
+    IfElse {
+        then_condition: ValueId,
+        then_value: ValueId,
+        else_condition: ValueId,
+        else_value: ValueId,
+    },
 }
 
 impl Instruction {
@@ -229,10 +239,12 @@ impl Instruction {
         match self {
             Instruction::Binary(binary) => binary.result_type(),
             Instruction::Cast(_, typ) => InstructionResultType::Known(typ.clone()),
-            Instruction::Not(value) | Instruction::Truncate { value, .. } => {
+            Instruction::Not(value)
+            | Instruction::Truncate { value, .. }
+            | Instruction::ArraySet { array: value, .. }
+            | Instruction::IfElse { then_value: value, .. } => {
                 InstructionResultType::Operand(*value)
             }
-            Instruction::ArraySet { array, .. } => InstructionResultType::Operand(*array),
             Instruction::Constrain(..)
             | Instruction::Store { .. }
             | Instruction::IncrementRc { .. }
@@ -252,20 +264,11 @@ impl Instruction {
         matches!(self.result_type(), InstructionResultType::Unknown)
     }
 
-    /// Pure `Instructions` are instructions which have no side-effects and results are a function of the inputs only,
-    /// i.e. there are no interactions with memory.
-    ///
-    /// Pure instructions can be replaced with the results of another pure instruction with the same inputs.
-    pub(crate) fn is_pure(&self, dfg: &DataFlowGraph) -> bool {
+    /// Indicates if the instruction can be safely replaced with the results of another instruction with the same inputs.
+    pub(crate) fn can_be_deduplicated(&self, dfg: &DataFlowGraph) -> bool {
         use Instruction::*;
 
         match self {
-            Binary(bin) => {
-                // In ACIR, a division with a false predicate outputs (0,0), so it cannot replace another instruction unless they have the same predicate
-                bin.operator != BinaryOp::Div
-            }
-            Cast(_, _) | Truncate { .. } | Not(_) => true,
-
             // These either have side-effects or interact with memory
             Constrain(..)
             | EnableSideEffects { .. }
@@ -276,31 +279,37 @@ impl Instruction {
             | DecrementRc { .. }
             | RangeCheck { .. } => false,
 
-            // These can have different behavior depending on the EnableSideEffectsIf context.
-            // Enabling constant folding for these potentially enables replacing an enabled
-            // array get with one that was disabled. See
-            // https://github.com/noir-lang/noir/pull/4716#issuecomment-2047846328.
-            ArrayGet { .. } | ArraySet { .. } => false,
-
             Call { func, .. } => match dfg[*func] {
                 Value::Intrinsic(intrinsic) => !intrinsic.has_side_effects(),
                 _ => false,
             },
+
+            // These can have different behavior depending on the EnableSideEffectsIf context.
+            // Replacing them with a similar instruction potentially enables replacing an instruction
+            // with one that was disabled. See
+            // https://github.com/noir-lang/noir/pull/4716#issuecomment-2047846328.
+            Binary(_)
+            | Cast(_, _)
+            | Not(_)
+            | Truncate { .. }
+            | IfElse { .. }
+            | ArrayGet { .. }
+            | ArraySet { .. } => !self.requires_acir_gen_predicate(dfg),
         }
     }
 
-    pub(crate) fn has_side_effects(&self, dfg: &DataFlowGraph) -> bool {
+    pub(crate) fn can_eliminate_if_unused(&self, dfg: &DataFlowGraph) -> 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) {
-                        rhs == FieldElement::zero()
+                        rhs != FieldElement::zero()
                     } else {
-                        true
+                        false
                     }
                 } else {
-                    false
+                    true
                 }
             }
             Cast(_, _)
@@ -309,32 +318,67 @@ impl Instruction {
             | Allocate
             | Load { .. }
             | ArrayGet { .. }
-            | ArraySet { .. } => false,
+            | IfElse { .. }
+            | ArraySet { .. } => true,
 
             Constrain(..)
             | Store { .. }
             | EnableSideEffects { .. }
             | IncrementRc { .. }
             | DecrementRc { .. }
-            | RangeCheck { .. } => true,
+            | RangeCheck { .. } => false,
 
             // Some `Intrinsic`s have side effects so we must check what kind of `Call` this is.
             Call { func, .. } => match dfg[*func] {
-                Value::Intrinsic(intrinsic) => intrinsic.has_side_effects(),
+                Value::Intrinsic(intrinsic) => !intrinsic.has_side_effects(),
 
                 // All foreign functions are treated as having side effects.
                 // This is because they can be used to pass information
                 // from the ACVM to the external world during execution.
-                Value::ForeignFunction(_) => true,
+                Value::ForeignFunction(_) => false,
 
                 // We must assume that functions contain a side effect as we cannot inspect more deeply.
-                Value::Function(_) => true,
+                Value::Function(_) => false,
 
                 _ => false,
             },
         }
     }
 
+    /// If true the instruction will depends on enable_side_effects context during acir-gen
+    fn requires_acir_gen_predicate(&self, dfg: &DataFlowGraph) -> bool {
+        match self {
+            Instruction::Binary(binary)
+                if matches!(binary.operator, BinaryOp::Div | BinaryOp::Mod) =>
+            {
+                true
+            }
+            Instruction::EnableSideEffects { .. }
+            | Instruction::ArrayGet { .. }
+            | Instruction::ArraySet { .. } => true,
+
+            Instruction::Call { func, .. } => match dfg[*func] {
+                Value::Function(_) => true,
+                Value::Intrinsic(intrinsic) => {
+                    matches!(intrinsic, Intrinsic::SliceInsert | Intrinsic::SliceRemove)
+                }
+                _ => false,
+            },
+            Instruction::Cast(_, _)
+            | Instruction::Binary(_)
+            | Instruction::Not(_)
+            | Instruction::Truncate { .. }
+            | Instruction::Constrain(_, _, _)
+            | Instruction::RangeCheck { .. }
+            | Instruction::Allocate
+            | Instruction::Load { .. }
+            | Instruction::Store { .. }
+            | Instruction::IfElse { .. }
+            | Instruction::IncrementRc { .. }
+            | Instruction::DecrementRc { .. } => false,
+        }
+    }
+
     /// Maps each ValueId inside this instruction to a new ValueId, returning the new instruction.
     /// Note that the returned instruction is fresh and will not have an assigned InstructionId
     /// until it is manually inserted in a DataFlowGraph later.
@@ -397,6 +441,14 @@ 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),
+                }
+            }
         }
     }
 
@@ -451,6 +503,12 @@ impl Instruction {
             | Instruction::RangeCheck { value, .. } => {
                 f(*value);
             }
+            Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
+                f(*then_condition);
+                f(*then_value);
+                f(*else_condition);
+                f(*else_value);
+            }
         }
     }
 
@@ -602,6 +660,36 @@ impl Instruction {
                     None
                 }
             }
+            Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
+                let typ = dfg.type_of_value(*then_value);
+
+                if let Some(constant) = dfg.get_numeric_constant(*then_condition) {
+                    if constant.is_one() {
+                        return SimplifiedTo(*then_value);
+                    } else if constant.is_zero() {
+                        return SimplifiedTo(*else_value);
+                    }
+                }
+
+                if matches!(&typ, Type::Numeric(_)) {
+                    let then_condition = *then_condition;
+                    let then_value = *then_value;
+                    let else_condition = *else_condition;
+                    let else_value = *else_value;
+
+                    let result = ValueMerger::merge_numeric_values(
+                        dfg,
+                        block,
+                        then_condition,
+                        else_condition,
+                        then_value,
+                        else_value,
+                    );
+                    SimplifiedTo(result)
+                } else {
+                    None
+                }
+            }
         }
     }
 }

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

@@ -354,7 +354,9 @@ fn simplify_slice_push_back(
     slice_sizes.insert(set_last_slice_value, slice_size / element_size);
     slice_sizes.insert(new_slice, slice_size / element_size);
 
-    let mut value_merger = ValueMerger::new(dfg, block, &mut slice_sizes);
+    let unknown = &mut HashMap::default();
+    let mut value_merger = ValueMerger::new(dfg, block, &mut slice_sizes, unknown, None);
+
     let new_slice = value_merger.merge_values(
         len_not_equals_capacity,
         len_equals_capacity,

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

@@ -181,7 +181,7 @@ fn display_instruction_inner(
             let index = show(*index);
             let value = show(*value);
             let mutable = if *mutable { " mut" } else { "" };
-            writeln!(f, "array_set{mutable} {array}, index {index}, value {value}",)
+            writeln!(f, "array_set{mutable} {array}, index {index}, value {value}")
         }
         Instruction::IncrementRc { value } => {
             writeln!(f, "inc_rc {}", show(*value))
@@ -192,6 +192,16 @@ fn display_instruction_inner(
         Instruction::RangeCheck { value, max_bit_size, .. } => {
             writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
         }
+        Instruction::IfElse { then_condition, then_value, else_condition, else_value } => {
+            let then_condition = show(*then_condition);
+            let then_value = show(*then_value);
+            let else_condition = show(*else_condition);
+            let else_value = show(*else_value);
+            writeln!(
+                f,
+                "if {then_condition} then {then_value} else if {else_condition} then {else_value}"
+            )
+        }
     }
 }