From 53f16c7fe75da04c54517b3d3199094b15195ce4 Mon Sep 17 00:00:00 2001
From: Akosh Farkash <aakoshh@gmail.com>
Date: Wed, 27 Nov 2024 22:50:59 +0000
Subject: [PATCH 1/6] feat(ssa): Deduplicate intrinsics with predicates (#6615)

---
 .../noirc_evaluator/src/ssa/ir/instruction.rs | 53 +++++++++++++++++--
 .../src/ssa/opt/constant_folding.rs           | 53 ++++++++++++++++++-
 2 files changed, 99 insertions(+), 7 deletions(-)

diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
index 9958aadd443..b48c755dbe5 100644
--- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
+++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
@@ -45,8 +45,7 @@ pub(crate) type InstructionId = Id<Instruction>;
 /// - Opcodes which the IR knows the target machine has
 /// special support for. (LowLevel)
 /// - Opcodes which have no function definition in the
-/// source code and must be processed by the IR. An example
-/// of this is println.
+/// source code and must be processed by the IR.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub(crate) enum Intrinsic {
     ArrayLen,
@@ -112,6 +111,9 @@ impl Intrinsic {
     /// Returns whether the `Intrinsic` has side effects.
     ///
     /// If there are no side effects then the `Intrinsic` can be removed if the result is unused.
+    ///
+    /// An example of a side effect is increasing the reference count of an array, but functions
+    /// which can fail due to implicit constraints are also considered to have a side effect.
     pub(crate) fn has_side_effects(&self) -> bool {
         match self {
             Intrinsic::AssertConstant
@@ -152,6 +154,39 @@ impl Intrinsic {
         }
     }
 
+    /// Intrinsics which only have a side effect due to the chance that
+    /// they can fail a constraint can be deduplicated.
+    pub(crate) fn can_be_deduplicated(&self, deduplicate_with_predicate: bool) -> bool {
+        match self {
+            // These apply a constraint in the form of ACIR opcodes, but they can be deduplicated
+            // if the inputs are the same. If they depend on a side effect variable (e.g. because
+            // they were in an if-then-else) then `handle_instruction_side_effects` in `flatten_cfg`
+            // will have attached the condition variable to their inputs directly, so they don't
+            // directly depend on the corresponding `enable_side_effect` instruction any more.
+            // However, to conform with the expectations of `Instruction::can_be_deduplicated` and
+            // `constant_folding` we only use this information if the caller shows interest in it.
+            Intrinsic::ToBits(_)
+            | Intrinsic::ToRadix(_)
+            | Intrinsic::BlackBox(
+                BlackBoxFunc::MultiScalarMul
+                | BlackBoxFunc::EmbeddedCurveAdd
+                | BlackBoxFunc::RecursiveAggregation,
+            ) => deduplicate_with_predicate,
+
+            // Operations that remove items from a slice don't modify the slice, they just assert it's non-empty.
+            Intrinsic::SlicePopBack | Intrinsic::SlicePopFront | Intrinsic::SliceRemove => {
+                deduplicate_with_predicate
+            }
+
+            Intrinsic::AssertConstant
+            | Intrinsic::StaticAssert
+            | Intrinsic::ApplyRangeConstraint
+            | Intrinsic::AsWitness => deduplicate_with_predicate,
+
+            _ => !self.has_side_effects(),
+        }
+    }
+
     /// Lookup an Intrinsic by name and return it if found.
     /// If there is no such intrinsic by that name, None is returned.
     pub(crate) fn lookup(name: &str) -> Option<Intrinsic> {
@@ -245,7 +280,7 @@ pub(crate) enum Instruction {
     /// - `code1` will have side effects iff `condition1` evaluates to `true`
     ///
     /// This instruction is only emitted after the cfg flattening pass, and is used to annotate
-    /// instruction regions with an condition that corresponds to their position in the CFG's
+    /// instruction regions with a condition that corresponds to their position in the CFG's
     /// if-branching structure.
     EnableSideEffectsIf { condition: ValueId },
 
@@ -331,6 +366,11 @@ impl Instruction {
     /// If `deduplicate_with_predicate` is set, we assume we're deduplicating with the instruction
     /// and its predicate, rather than just the instruction. Setting this means instructions that
     /// rely on predicates can be deduplicated as well.
+    ///
+    /// Some instructions get the predicate attached to their inputs by `handle_instruction_side_effects` in `flatten_cfg`.
+    /// These can be deduplicated because they implicitly depend on the predicate, not only when the caller uses the
+    /// predicate variable as a key to cache results. However, to avoid tight coupling between passes, we make the deduplication
+    /// conditional on whether the caller wants the predicate to be taken into account or not.
     pub(crate) fn can_be_deduplicated(
         &self,
         dfg: &DataFlowGraph,
@@ -348,7 +388,9 @@ impl Instruction {
             | DecrementRc { .. } => false,
 
             Call { func, .. } => match dfg[*func] {
-                Value::Intrinsic(intrinsic) => !intrinsic.has_side_effects(),
+                Value::Intrinsic(intrinsic) => {
+                    intrinsic.can_be_deduplicated(deduplicate_with_predicate)
+                }
                 _ => false,
             },
 
@@ -421,6 +463,7 @@ impl Instruction {
                 // 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,
+
                 Value::Intrinsic(intrinsic) => !intrinsic.has_side_effects(),
 
                 // All foreign functions are treated as having side effects.
@@ -436,7 +479,7 @@ impl Instruction {
         }
     }
 
-    /// If true the instruction will depends on enable_side_effects context during acir-gen
+    /// If true the instruction will depend on `enable_side_effects` context during acir-gen.
     pub(crate) fn requires_acir_gen_predicate(&self, dfg: &DataFlowGraph) -> bool {
         match self {
             Instruction::Binary(binary)
diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
index 019bace33a3..96683804042 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
@@ -6,7 +6,7 @@
 //!   by the [`DataFlowGraph`] automatically as new instructions are pushed.
 //! - Check whether any input values have been constrained to be equal to a value of a simpler form
 //!   by a [constrain instruction][Instruction::Constrain]. If so, replace the input value with the simpler form.
-//! - Check whether the instruction [can_be_replaced][Instruction::can_be_replaced()]
+//! - Check whether the instruction [can_be_deduplicated][Instruction::can_be_deduplicated()]
 //!   by duplicate instruction earlier in the same block.
 //!
 //! These operations are done in parallel so that they can each benefit from each other
@@ -54,6 +54,9 @@ use fxhash::FxHashMap as HashMap;
 impl Ssa {
     /// Performs constant folding on each instruction.
     ///
+    /// It will not look at constraints to inform simplifications
+    /// based on the stated equivalence of two instructions.
+    ///
     /// See [`constant_folding`][self] module for more information.
     #[tracing::instrument(level = "trace", skip(self))]
     pub(crate) fn fold_constants(mut self) -> Ssa {
@@ -188,9 +191,12 @@ pub(crate) struct BrilligInfo<'a> {
     brillig_functions: &'a BTreeMap<FunctionId, Function>,
 }
 
-/// HashMap from (Instruction, side_effects_enabled_var) to the results of the instruction.
+/// HashMap from `(Instruction, side_effects_enabled_var)` to the results of the instruction.
 /// Stored as a two-level map to avoid cloning Instructions during the `.get` call.
 ///
+/// The `side_effects_enabled_var` is optional because we only use them when `Instruction::requires_acir_gen_predicate`
+/// is true _and_ the constraint information is also taken into account.
+///
 /// In addition to each result, the original BasicBlockId is stored as well. This allows us
 /// to deduplicate instructions across blocks as long as the new block dominates the original.
 type InstructionResultCache = HashMap<Instruction, HashMap<Option<ValueId>, ResultCache>>;
@@ -223,6 +229,7 @@ impl<'brillig> Context<'brillig> {
     fn fold_constants_in_block(&mut self, function: &mut Function, block: BasicBlockId) {
         let instructions = function.dfg[block].take_instructions();
 
+        // Default side effect condition variable with an enabled state.
         let mut side_effects_enabled_var =
             function.dfg.make_constant(FieldElement::one(), Type::bool());
 
@@ -403,6 +410,7 @@ impl<'brillig> Context<'brillig> {
 
         // If the instruction doesn't have side-effects and if it won't interact with enable_side_effects during acir_gen,
         // we cache the results so we can reuse them if the same instruction appears again later in the block.
+        // Others have side effects representing failure, which are implicit in the ACIR code and can also be deduplicated.
         if instruction.can_be_deduplicated(dfg, self.use_constraint_info) {
             let use_predicate =
                 self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg);
@@ -417,6 +425,8 @@ impl<'brillig> Context<'brillig> {
         }
     }
 
+    /// Get the simplification mapping from complex to simpler instructions,
+    /// which all depend on the same side effect condition variable.
     fn get_constraint_map(
         &mut self,
         side_effects_enabled_var: ValueId,
@@ -1341,4 +1351,43 @@ mod test {
         let ssa = ssa.fold_constants_with_brillig(&brillig);
         assert_normalized_ssa_equals(ssa, expected);
     }
+
+    #[test]
+    fn deduplicates_side_effecting_intrinsics() {
+        let src = "
+        // After EnableSideEffectsIf removal:
+        acir(inline) fn main f0 {
+          b0(v0: Field, v1: Field, v2: u1):
+            v4 = call is_unconstrained() -> u1
+            v7 = call to_be_radix(v0, u32 256) -> [u8; 1]    // `a.to_be_radix(256)`;
+            inc_rc v7
+            v8 = call to_be_radix(v0, u32 256) -> [u8; 1]    // duplicate load of `a`
+            inc_rc v8
+            v9 = cast v2 as Field                            // `if c { a.to_be_radix(256) }`
+            v10 = mul v0, v9                                 // attaching `c` to `a`
+            v11 = call to_be_radix(v10, u32 256) -> [u8; 1]  // calling `to_radix(c * a)`
+            inc_rc v11
+            enable_side_effects v2                           // side effect var for `c` shifted down by removal
+            return
+        }
+        ";
+        let ssa = Ssa::from_str(src).unwrap();
+        let expected = "
+        acir(inline) fn main f0 {
+          b0(v0: Field, v1: Field, v2: u1):
+            v4 = call is_unconstrained() -> u1
+            v7 = call to_be_radix(v0, u32 256) -> [u8; 1]
+            inc_rc v7
+            inc_rc v7
+            v8 = cast v2 as Field
+            v9 = mul v0, v8
+            v10 = call to_be_radix(v9, u32 256) -> [u8; 1]
+            inc_rc v10
+            enable_side_effects v2
+            return
+        }
+        ";
+        let ssa = ssa.fold_constants_using_constraints();
+        assert_normalized_ssa_equals(ssa, expected);
+    }
 }

From dfc9ff7266d2b6694cae3da88418013664440daa Mon Sep 17 00:00:00 2001
From: Tom French <15848336+TomAFrench@users.noreply.github.com>
Date: Thu, 28 Nov 2024 12:01:05 +0000
Subject: [PATCH 2/6] chore: pin foundry version in CI (#6642)

---
 .github/workflows/test-js-packages.yml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml
index 152d8b1653e..4a5d0b8179b 100644
--- a/.github/workflows/test-js-packages.yml
+++ b/.github/workflows/test-js-packages.yml
@@ -478,6 +478,9 @@ jobs:
 
       - name: Install Foundry
         uses: foundry-rs/foundry-toolchain@v1.2.0
+        with:
+          version: nightly-8660e5b941fe7f4d67e246cfd3dafea330fb53b1
+          
 
       - name: Install `bb`
         run: |

From b0245811bfd84e0bf3559aa1e2f37ec52d08691e Mon Sep 17 00:00:00 2001
From: Ary Borenszweig <asterite@gmail.com>
Date: Thu, 28 Nov 2024 11:26:57 -0300
Subject: [PATCH 3/6] fix(ssa): don't deduplicate constraints in blocks that
 are not dominated (#6627)

Co-authored-by: Akosh Farkash <aakoshh@gmail.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
---
 compiler/noirc_evaluator/src/ssa/ir/dom.rs    |  41 ++++
 .../noirc_evaluator/src/ssa/ir/instruction.rs |  38 +++
 .../src/ssa/opt/constant_folding.rs           | 225 +++++++++++++-----
 3 files changed, 250 insertions(+), 54 deletions(-)

diff --git a/compiler/noirc_evaluator/src/ssa/ir/dom.rs b/compiler/noirc_evaluator/src/ssa/ir/dom.rs
index c1a7f14e0d1..504eecf4801 100644
--- a/compiler/noirc_evaluator/src/ssa/ir/dom.rs
+++ b/compiler/noirc_evaluator/src/ssa/ir/dom.rs
@@ -119,6 +119,29 @@ impl DominatorTree {
         }
     }
 
+    /// Walk up the dominator tree until we find a block for which `f` returns `Some` value.
+    /// Otherwise return `None` when we reach the top.
+    ///
+    /// Similar to `Iterator::filter_map` but only returns the first hit.
+    pub(crate) fn find_map_dominator<T>(
+        &self,
+        mut block_id: BasicBlockId,
+        f: impl Fn(BasicBlockId) -> Option<T>,
+    ) -> Option<T> {
+        if !self.is_reachable(block_id) {
+            return None;
+        }
+        loop {
+            if let Some(value) = f(block_id) {
+                return Some(value);
+            }
+            block_id = match self.immediate_dominator(block_id) {
+                Some(immediate_dominator) => immediate_dominator,
+                None => return None,
+            }
+        }
+    }
+
     /// Allocate and compute a dominator tree from a pre-computed control flow graph and
     /// post-order counterpart.
     pub(crate) fn with_cfg_and_post_order(cfg: &ControlFlowGraph, post_order: &PostOrder) -> Self {
@@ -448,4 +471,22 @@ mod tests {
         assert!(dt.dominates(block2_id, block1_id));
         assert!(dt.dominates(block2_id, block2_id));
     }
+
+    #[test]
+    fn test_find_map_dominator() {
+        let (dt, b0, b1, b2, _b3) = unreachable_node_setup();
+
+        assert_eq!(
+            dt.find_map_dominator(b2, |b| if b == b0 { Some("root") } else { None }),
+            Some("root")
+        );
+        assert_eq!(
+            dt.find_map_dominator(b1, |b| if b == b0 { Some("unreachable") } else { None }),
+            None
+        );
+        assert_eq!(
+            dt.find_map_dominator(b1, |b| if b == b1 { Some("not part of tree") } else { None }),
+            None
+        );
+    }
 }
diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
index b48c755dbe5..6737b335b7d 100644
--- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
+++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
@@ -362,6 +362,44 @@ impl Instruction {
         matches!(self.result_type(), InstructionResultType::Unknown)
     }
 
+    /// Indicates if the instruction has a side effect, ie. it can fail, or it interacts with memory.
+    ///
+    /// This is similar to `can_be_deduplicated`, but it doesn't depend on whether the caller takes
+    /// constraints into account, because it might not use it to isolate the side effects across branches.
+    pub(crate) fn has_side_effects(&self, dfg: &DataFlowGraph) -> bool {
+        use Instruction::*;
+
+        match self {
+            // These either have side-effects or interact with memory
+            EnableSideEffectsIf { .. }
+            | Allocate
+            | Load { .. }
+            | Store { .. }
+            | IncrementRc { .. }
+            | DecrementRc { .. } => true,
+
+            Call { func, .. } => match dfg[*func] {
+                Value::Intrinsic(intrinsic) => intrinsic.has_side_effects(),
+                _ => true, // Be conservative and assume other functions can have side effects.
+            },
+
+            // These can fail.
+            Constrain(..) | RangeCheck { .. } => true,
+
+            // This should never be side-effectful
+            MakeArray { .. } => false,
+
+            // These can have different behavior depending on the EnableSideEffectsIf context.
+            Binary(_)
+            | Cast(_, _)
+            | Not(_)
+            | Truncate { .. }
+            | IfElse { .. }
+            | ArrayGet { .. }
+            | ArraySet { .. } => self.requires_acir_gen_predicate(dfg),
+        }
+    }
+
     /// Indicates if the instruction can be safely replaced with the results of another instruction with the same inputs.
     /// If `deduplicate_with_predicate` is set, we assume we're deduplicating with the instruction
     /// and its predicate, rather than just the instruction. Setting this means instructions that
diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
index 96683804042..7236dba76db 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
@@ -149,7 +149,8 @@ impl Function {
         use_constraint_info: bool,
         brillig_info: Option<BrilligInfo>,
     ) {
-        let mut context = Context::new(self, use_constraint_info, brillig_info);
+        let mut context = Context::new(use_constraint_info, brillig_info);
+        let mut dom = DominatorTree::with_function(self);
         context.block_queue.push_back(self.entry_block());
 
         while let Some(block) = context.block_queue.pop_front() {
@@ -158,7 +159,7 @@ impl Function {
             }
 
             context.visited_blocks.insert(block);
-            context.fold_constants_in_block(self, block);
+            context.fold_constants_in_block(&mut self.dfg, &mut dom, block);
         }
     }
 }
@@ -177,12 +178,10 @@ struct Context<'a> {
     /// We partition the maps of constrained values according to the side-effects flag at the point
     /// at which the values are constrained. This prevents constraints which are only sometimes enforced
     /// being used to modify the rest of the program.
-    constraint_simplification_mappings: HashMap<ValueId, HashMap<ValueId, ValueId>>,
+    constraint_simplification_mappings: ConstraintSimplificationCache,
 
     // Cache of instructions without any side-effects along with their outputs.
     cached_instruction_results: InstructionResultCache,
-
-    dom: DominatorTree,
 }
 
 #[derive(Copy, Clone)]
@@ -191,6 +190,50 @@ pub(crate) struct BrilligInfo<'a> {
     brillig_functions: &'a BTreeMap<FunctionId, Function>,
 }
 
+/// Records a simplified equivalents of an [`Instruction`] in the blocks
+/// where the constraint that advised the simplification has been encountered.
+///
+/// For more information see [`ConstraintSimplificationCache`].
+#[derive(Default)]
+struct SimplificationCache {
+    /// Simplified expressions where we found them.
+    ///
+    /// It will always have at least one value because `add` is called
+    /// after the default is constructed.
+    simplifications: HashMap<BasicBlockId, ValueId>,
+}
+
+impl SimplificationCache {
+    /// Called with a newly encountered simplification.
+    fn add(&mut self, dfg: &DataFlowGraph, simple: ValueId, block: BasicBlockId) {
+        self.simplifications
+            .entry(block)
+            .and_modify(|existing| {
+                // `SimplificationCache` may already hold a simplification in this block
+                // so we check whether `simple` is a better simplification than the current one.
+                if let Some((_, simpler)) = simplify(dfg, *existing, simple) {
+                    *existing = simpler;
+                };
+            })
+            .or_insert(simple);
+    }
+
+    /// Try to find a simplification in a visible block.
+    fn get(&self, block: BasicBlockId, dom: &DominatorTree) -> Option<ValueId> {
+        // Deterministically walk up the dominator chain until we encounter a block that contains a simplification.
+        dom.find_map_dominator(block, |b| self.simplifications.get(&b).cloned())
+    }
+}
+
+/// HashMap from `(side_effects_enabled_var, Instruction)` to a simplified expression that it can
+/// be replaced with based on constraints that testify to their equivalence, stored together
+/// with the set of blocks at which this constraint has been observed.
+///
+/// Only blocks dominated by one in the cache should have access to this information, otherwise
+/// we create a sort of time paradox where we replace an instruction with a constant we believe
+/// it _should_ equal to, without ever actually producing and asserting the value.
+type ConstraintSimplificationCache = HashMap<ValueId, HashMap<ValueId, SimplificationCache>>;
+
 /// HashMap from `(Instruction, side_effects_enabled_var)` to the results of the instruction.
 /// Stored as a two-level map to avoid cloning Instructions during the `.get` call.
 ///
@@ -210,11 +253,7 @@ struct ResultCache {
 }
 
 impl<'brillig> Context<'brillig> {
-    fn new(
-        function: &Function,
-        use_constraint_info: bool,
-        brillig_info: Option<BrilligInfo<'brillig>>,
-    ) -> Self {
+    fn new(use_constraint_info: bool, brillig_info: Option<BrilligInfo<'brillig>>) -> Self {
         Self {
             use_constraint_info,
             brillig_info,
@@ -222,49 +261,57 @@ impl<'brillig> Context<'brillig> {
             block_queue: Default::default(),
             constraint_simplification_mappings: Default::default(),
             cached_instruction_results: Default::default(),
-            dom: DominatorTree::with_function(function),
         }
     }
 
-    fn fold_constants_in_block(&mut self, function: &mut Function, block: BasicBlockId) {
-        let instructions = function.dfg[block].take_instructions();
+    fn fold_constants_in_block(
+        &mut self,
+        dfg: &mut DataFlowGraph,
+        dom: &mut DominatorTree,
+        block: BasicBlockId,
+    ) {
+        let instructions = dfg[block].take_instructions();
 
         // Default side effect condition variable with an enabled state.
-        let mut side_effects_enabled_var =
-            function.dfg.make_constant(FieldElement::one(), Type::bool());
+        let mut side_effects_enabled_var = dfg.make_constant(FieldElement::one(), Type::bool());
 
         for instruction_id in instructions {
             self.fold_constants_into_instruction(
-                &mut function.dfg,
+                dfg,
+                dom,
                 block,
                 instruction_id,
                 &mut side_effects_enabled_var,
             );
         }
-        self.block_queue.extend(function.dfg[block].successors());
+        self.block_queue.extend(dfg[block].successors());
     }
 
     fn fold_constants_into_instruction(
         &mut self,
         dfg: &mut DataFlowGraph,
+        dom: &mut DominatorTree,
         mut block: BasicBlockId,
         id: InstructionId,
         side_effects_enabled_var: &mut ValueId,
     ) {
         let constraint_simplification_mapping = self.get_constraint_map(*side_effects_enabled_var);
-        let instruction = Self::resolve_instruction(id, dfg, constraint_simplification_mapping);
+
+        let instruction =
+            Self::resolve_instruction(id, block, dfg, dom, constraint_simplification_mapping);
+
         let old_results = dfg.instruction_results(id).to_vec();
 
         // If a copy of this instruction exists earlier in the block, then reuse the previous results.
         if let Some(cache_result) =
-            self.get_cached(dfg, &instruction, *side_effects_enabled_var, block)
+            self.get_cached(dfg, dom, &instruction, *side_effects_enabled_var, block)
         {
             match cache_result {
                 CacheResult::Cached(cached) => {
                     Self::replace_result_ids(dfg, &old_results, cached);
                     return;
                 }
-                CacheResult::NeedToHoistToCommonBlock(dominator, _cached) => {
+                CacheResult::NeedToHoistToCommonBlock(dominator) => {
                     // Just change the block to insert in the common dominator instead.
                     // This will only move the current instance of the instruction right now.
                     // When constant folding is run a second time later on, it'll catch
@@ -314,8 +361,10 @@ impl<'brillig> Context<'brillig> {
     /// Fetches an [`Instruction`] by its [`InstructionId`] and fully resolves its inputs.
     fn resolve_instruction(
         instruction_id: InstructionId,
+        block: BasicBlockId,
         dfg: &DataFlowGraph,
-        constraint_simplification_mapping: &HashMap<ValueId, ValueId>,
+        dom: &mut DominatorTree,
+        constraint_simplification_mapping: &HashMap<ValueId, SimplificationCache>,
     ) -> Instruction {
         let instruction = dfg[instruction_id].clone();
 
@@ -325,20 +374,29 @@ impl<'brillig> Context<'brillig> {
         // This allows us to reach a stable final `ValueId` for each instruction input as we add more
         // constraints to the cache.
         fn resolve_cache(
+            block: BasicBlockId,
             dfg: &DataFlowGraph,
-            cache: &HashMap<ValueId, ValueId>,
+            dom: &mut DominatorTree,
+            cache: &HashMap<ValueId, SimplificationCache>,
             value_id: ValueId,
         ) -> ValueId {
             let resolved_id = dfg.resolve(value_id);
             match cache.get(&resolved_id) {
-                Some(cached_value) => resolve_cache(dfg, cache, *cached_value),
+                Some(simplification_cache) => {
+                    if let Some(simplified) = simplification_cache.get(block, dom) {
+                        resolve_cache(block, dfg, dom, cache, simplified)
+                    } else {
+                        resolved_id
+                    }
+                }
                 None => resolved_id,
             }
         }
 
         // Resolve any inputs to ensure that we're comparing like-for-like instructions.
-        instruction
-            .map_values(|value_id| resolve_cache(dfg, constraint_simplification_mapping, value_id))
+        instruction.map_values(|value_id| {
+            resolve_cache(block, dfg, dom, constraint_simplification_mapping, value_id)
+        })
     }
 
     /// Pushes a new [`Instruction`] into the [`DataFlowGraph`] which applies any optimizations
@@ -384,26 +442,11 @@ impl<'brillig> Context<'brillig> {
             // to map from the more complex to the simpler value.
             if let Instruction::Constrain(lhs, rhs, _) = instruction {
                 // These `ValueId`s should be fully resolved now.
-                match (&dfg[lhs], &dfg[rhs]) {
-                    // Ignore trivial constraints
-                    (Value::NumericConstant { .. }, Value::NumericConstant { .. }) => (),
-
-                    // Prefer replacing with constants where possible.
-                    (Value::NumericConstant { .. }, _) => {
-                        self.get_constraint_map(side_effects_enabled_var).insert(rhs, lhs);
-                    }
-                    (_, Value::NumericConstant { .. }) => {
-                        self.get_constraint_map(side_effects_enabled_var).insert(lhs, rhs);
-                    }
-                    // Otherwise prefer block parameters over instruction results.
-                    // This is as block parameters are more likely to be a single witness rather than a full expression.
-                    (Value::Param { .. }, Value::Instruction { .. }) => {
-                        self.get_constraint_map(side_effects_enabled_var).insert(rhs, lhs);
-                    }
-                    (Value::Instruction { .. }, Value::Param { .. }) => {
-                        self.get_constraint_map(side_effects_enabled_var).insert(lhs, rhs);
-                    }
-                    (_, _) => (),
+                if let Some((complex, simple)) = simplify(dfg, lhs, rhs) {
+                    self.get_constraint_map(side_effects_enabled_var)
+                        .entry(complex)
+                        .or_default()
+                        .add(dfg, simple, block);
                 }
             }
         }
@@ -430,7 +473,7 @@ impl<'brillig> Context<'brillig> {
     fn get_constraint_map(
         &mut self,
         side_effects_enabled_var: ValueId,
-    ) -> &mut HashMap<ValueId, ValueId> {
+    ) -> &mut HashMap<ValueId, SimplificationCache> {
         self.constraint_simplification_mappings.entry(side_effects_enabled_var).or_default()
     }
 
@@ -445,9 +488,11 @@ impl<'brillig> Context<'brillig> {
         }
     }
 
+    /// Get a cached result if it can be used in this context.
     fn get_cached(
-        &mut self,
+        &self,
         dfg: &DataFlowGraph,
+        dom: &mut DominatorTree,
         instruction: &Instruction,
         side_effects_enabled_var: ValueId,
         block: BasicBlockId,
@@ -457,7 +502,7 @@ impl<'brillig> Context<'brillig> {
         let predicate = self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg);
         let predicate = predicate.then_some(side_effects_enabled_var);
 
-        results_for_instruction.get(&predicate)?.get(block, &mut self.dom)
+        results_for_instruction.get(&predicate)?.get(block, dom, instruction.has_side_effects(dfg))
     }
 
     /// Checks if the given instruction is a call to a brillig function with all constant arguments.
@@ -635,14 +680,21 @@ impl ResultCache {
     /// We require that the cached instruction's block dominates `block` in order to avoid
     /// cycles causing issues (e.g. two instructions being replaced with the results of each other
     /// such that neither instruction exists anymore.)
-    fn get(&self, block: BasicBlockId, dom: &mut DominatorTree) -> Option<CacheResult> {
-        self.result.as_ref().map(|(origin_block, results)| {
+    fn get(
+        &self,
+        block: BasicBlockId,
+        dom: &mut DominatorTree,
+        has_side_effects: bool,
+    ) -> Option<CacheResult> {
+        self.result.as_ref().and_then(|(origin_block, results)| {
             if dom.dominates(*origin_block, block) {
-                CacheResult::Cached(results)
-            } else {
+                Some(CacheResult::Cached(results))
+            } else if !has_side_effects {
                 // Insert a copy of this instruction in the common dominator
                 let dominator = dom.common_dominator(*origin_block, block);
-                CacheResult::NeedToHoistToCommonBlock(dominator, results)
+                Some(CacheResult::NeedToHoistToCommonBlock(dominator))
+            } else {
+                None
             }
         })
     }
@@ -650,7 +702,7 @@ impl ResultCache {
 
 enum CacheResult<'a> {
     Cached(&'a [ValueId]),
-    NeedToHoistToCommonBlock(BasicBlockId, &'a [ValueId]),
+    NeedToHoistToCommonBlock(BasicBlockId),
 }
 
 /// Result of trying to evaluate an instruction (any instruction) in this pass.
@@ -697,6 +749,25 @@ fn value_id_to_calldata(value_id: ValueId, dfg: &DataFlowGraph, calldata: &mut V
     panic!("Expected ValueId to be numeric constant or array constant");
 }
 
+/// Check if one expression is simpler than the other.
+/// Returns `Some((complex, simple))` if a simplification was found, otherwise `None`.
+/// Expects the `ValueId`s to be fully resolved.
+fn simplify(dfg: &DataFlowGraph, lhs: ValueId, rhs: ValueId) -> Option<(ValueId, ValueId)> {
+    match (&dfg[lhs], &dfg[rhs]) {
+        // Ignore trivial constraints
+        (Value::NumericConstant { .. }, Value::NumericConstant { .. }) => None,
+
+        // Prefer replacing with constants where possible.
+        (Value::NumericConstant { .. }, _) => Some((rhs, lhs)),
+        (_, Value::NumericConstant { .. }) => Some((lhs, rhs)),
+        // Otherwise prefer block parameters over instruction results.
+        // This is as block parameters are more likely to be a single witness rather than a full expression.
+        (Value::Param { .. }, Value::Instruction { .. }) => Some((rhs, lhs)),
+        (Value::Instruction { .. }, Value::Param { .. }) => Some((lhs, rhs)),
+        (_, _) => None,
+    }
+}
+
 #[cfg(test)]
 mod test {
     use std::sync::Arc;
@@ -1352,6 +1423,52 @@ mod test {
         assert_normalized_ssa_equals(ssa, expected);
     }
 
+    #[test]
+    fn does_not_use_cached_constrain_in_block_that_is_not_dominated() {
+        let src = "
+            brillig(inline) fn main f0 {
+              b0(v0: Field, v1: Field):
+                v3 = eq v0, Field 0
+                jmpif v3 then: b1, else: b2
+              b1():
+                v5 = eq v1, Field 1
+                constrain v1 == Field 1
+                jmp b2()
+              b2():
+                v6 = eq v1, Field 0
+                constrain v1 == Field 0
+                return
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+        let ssa = ssa.fold_constants_using_constraints();
+        assert_normalized_ssa_equals(ssa, src);
+    }
+
+    #[test]
+    fn does_not_hoist_constrain_to_common_ancestor() {
+        let src = "
+            brillig(inline) fn main f0 {
+              b0(v0: Field, v1: Field):
+                v3 = eq v0, Field 0
+                jmpif v3 then: b1, else: b2
+              b1():
+                constrain v1 == Field 1
+                jmp b2()
+              b2():
+                jmpif v0 then: b3, else: b4
+              b3():
+                constrain v1 == Field 1 // This was incorrectly hoisted to b0 but this condition is not valid when going b0 -> b2 -> b4
+                jmp b4()
+              b4():
+                return
+            }
+            ";
+        let ssa = Ssa::from_str(src).unwrap();
+        let ssa = ssa.fold_constants_using_constraints();
+        assert_normalized_ssa_equals(ssa, src);
+    }
+
     #[test]
     fn deduplicates_side_effecting_intrinsics() {
         let src = "

From dace07849aa28795abb30b3f9d979ffc6b6487e6 Mon Sep 17 00:00:00 2001
From: guipublic <47281315+guipublic@users.noreply.github.com>
Date: Thu, 28 Nov 2024 18:05:06 +0100
Subject: [PATCH 4/6] fix: used signed division for signed modulo (#6635)

---
 .../noirc_evaluator/src/acir/acir_variable.rs   | 17 ++++++++++++++---
 compiler/noirc_evaluator/src/acir/mod.rs        |  1 +
 docs/docs/noir/concepts/data_types/integers.md  | 11 +++++++++++
 .../execution_success/regression/Prover.toml    |  2 ++
 .../execution_success/regression/src/main.nr    | 10 +++++++++-
 5 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/compiler/noirc_evaluator/src/acir/acir_variable.rs b/compiler/noirc_evaluator/src/acir/acir_variable.rs
index 6ba072f01a4..a42426e6c04 100644
--- a/compiler/noirc_evaluator/src/acir/acir_variable.rs
+++ b/compiler/noirc_evaluator/src/acir/acir_variable.rs
@@ -578,7 +578,7 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
         let numeric_type = match typ {
             AcirType::NumericType(numeric_type) => numeric_type,
             AcirType::Array(_, _) => {
-                todo!("cannot divide arrays. This should have been caught by the frontend")
+                unreachable!("cannot divide arrays. This should have been caught by the frontend")
             }
         };
         match numeric_type {
@@ -1084,11 +1084,22 @@ impl<F: AcirField, B: BlackBoxFunctionSolver<F>> AcirContext<F, B> {
         &mut self,
         lhs: AcirVar,
         rhs: AcirVar,
+        typ: AcirType,
         bit_size: u32,
         predicate: AcirVar,
     ) -> Result<AcirVar, RuntimeError> {
-        let (_, remainder) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?;
-        Ok(remainder)
+        let numeric_type = match typ {
+            AcirType::NumericType(numeric_type) => numeric_type,
+            AcirType::Array(_, _) => {
+                unreachable!("cannot modulo arrays. This should have been caught by the frontend")
+            }
+        };
+
+        let (_, remainder_var) = match numeric_type {
+            NumericType::Signed { bit_size } => self.signed_division_var(lhs, rhs, bit_size)?,
+            _ => self.euclidean_division_var(lhs, rhs, bit_size, predicate)?,
+        };
+        Ok(remainder_var)
     }
 
     /// Constrains the `AcirVar` variable to be of type `NumericType`.
diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs
index 7274fe908d1..69679495b92 100644
--- a/compiler/noirc_evaluator/src/acir/mod.rs
+++ b/compiler/noirc_evaluator/src/acir/mod.rs
@@ -1968,6 +1968,7 @@ impl<'a> Context<'a> {
             BinaryOp::Mod => self.acir_context.modulo_var(
                 lhs,
                 rhs,
+                binary_type.clone(),
                 bit_count,
                 self.current_side_effects_enabled_var,
             ),
diff --git a/docs/docs/noir/concepts/data_types/integers.md b/docs/docs/noir/concepts/data_types/integers.md
index a1d59bf3166..f3badde62be 100644
--- a/docs/docs/noir/concepts/data_types/integers.md
+++ b/docs/docs/noir/concepts/data_types/integers.md
@@ -47,6 +47,17 @@ fn main() {
 
 The bit size determines the maximum and minimum range of value the integer type can store. For example, an `i8` variable can store a value in the range of -128 to 127 (i.e. $\\-2^{7}\\$ to $\\2^{7}-1\\$).
 
+
+```rust
+fn main(x: i16, y: i16) {
+    // modulo
+    let c = x % y;
+    let c = x % -13;
+}
+```
+
+Modulo operation is defined for negative integers thanks to integer division, so that the equality `x = (x/y)*y + (x%y)` holds.
+
 ## 128 bits Unsigned Integers
 
 The built-in structure `U128` allows you to use 128-bit unsigned integers almost like a native integer type. However, there are some differences to keep in mind:
diff --git a/test_programs/execution_success/regression/Prover.toml b/test_programs/execution_success/regression/Prover.toml
index 2875190982f..c81cbf10fbb 100644
--- a/test_programs/execution_success/regression/Prover.toml
+++ b/test_programs/execution_success/regression/Prover.toml
@@ -1,2 +1,4 @@
 x = [0x3f, 0x1c, 0xb8, 0x99, 0xab]
 z = 3
+u = "169"
+v = "-13"
\ No newline at end of file
diff --git a/test_programs/execution_success/regression/src/main.nr b/test_programs/execution_success/regression/src/main.nr
index 1c2f557d2cd..809fdbe4b28 100644
--- a/test_programs/execution_success/regression/src/main.nr
+++ b/test_programs/execution_success/regression/src/main.nr
@@ -94,7 +94,7 @@ fn bitshift_variable(idx: u8) -> u64 {
     bits
 }
 
-fn main(x: [u8; 5], z: Field) {
+fn main(x: [u8; 5], z: Field, u: i16, v: i16) {
     //Issue 1144
     let (nib, len) = compact_decode(x, z);
     assert(len == 5);
@@ -130,4 +130,12 @@ fn main(x: [u8; 5], z: Field) {
     assert(result_0 == 1);
     let result_4 = bitshift_variable(4);
     assert(result_4 == 16);
+
+    // Issue 6609
+    assert(u % -13 == 0);
+    assert(u % v == 0);
+    assert(u % -11 == 4);
+    assert(-u % -11 == -4);
+    assert(u % -11 == u % (v + 2));
+    assert(-u % -11 == -u % (v + 2));
 }

From aa143a75d3460785ed88ea7ab3337c880c1153fd Mon Sep 17 00:00:00 2001
From: Aztec Bot <49558828+AztecBot@users.noreply.github.com>
Date: Thu, 28 Nov 2024 13:24:31 -0500
Subject: [PATCH 5/6] feat: Sync from aztec-packages (#6634)

Co-authored-by: TomAFrench <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
---
 .aztec-sync-commit                            |  2 +-
 Cargo.lock                                    |  4 +--
 .../src/ssa/opt/constant_folding.rs           | 36 +++++++------------
 .../src/ssa/opt/flatten_cfg.rs                |  6 +---
 4 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/.aztec-sync-commit b/.aztec-sync-commit
index 063664dceac..d97a936c081 100644
--- a/.aztec-sync-commit
+++ b/.aztec-sync-commit
@@ -1 +1 @@
-b8bace9a00c3a8eb93f42682e8cbfa351fc5238c
+1bfc15e08873a1f0f3743e259f418b70426b3f25
diff --git a/Cargo.lock b/Cargo.lock
index aacd8f7e596..94a84b89d05 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -805,9 +805,9 @@ dependencies = [
 
 [[package]]
 name = "clap_complete"
-version = "4.5.38"
+version = "4.5.37"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d9647a559c112175f17cf724dc72d3645680a883c58481332779192b0d8e7a01"
+checksum = "11611dca53440593f38e6b25ec629de50b14cdfa63adc0fb856115a2c6d97595"
 dependencies = [
  "clap",
 ]
diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
index 7236dba76db..41c84c935b1 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
@@ -319,7 +319,7 @@ impl<'brillig> Context<'brillig> {
                     block = dominator;
                 }
             }
-        }
+        };
 
         let new_results =
         // First try to inline a call to a brillig function with all constant arguments.
@@ -498,7 +498,6 @@ impl<'brillig> Context<'brillig> {
         block: BasicBlockId,
     ) -> Option<CacheResult> {
         let results_for_instruction = self.cached_instruction_results.get(instruction)?;
-
         let predicate = self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg);
         let predicate = predicate.then_some(side_effects_enabled_var);
 
@@ -1004,32 +1003,22 @@ mod test {
     // Regression for #4600
     #[test]
     fn array_get_regression() {
-        // fn main f0 {
-        //   b0(v0: u1, v1: u64):
-        //     enable_side_effects_if v0
-        //     v2 = make_array [Field 0, Field 1]
-        //     v3 = array_get v2, index v1
-        //     v4 = not v0
-        //     enable_side_effects_if v4
-        //     v5 = array_get v2, index v1
-        // }
-        //
         // We want to make sure after constant folding both array_gets remain since they are
         // under different enable_side_effects_if contexts and thus one may be disabled while
         // the other is not. If one is removed, it is possible e.g. v4 is replaced with v2 which
         // is disabled (only gets from index 0) and thus returns the wrong result.
         let src = "
-            acir(inline) fn main f0 {
-              b0(v0: u1, v1: u64):
-                enable_side_effects v0
-                v4 = make_array [Field 0, Field 1] : [Field; 2]
-                v5 = array_get v4, index v1 -> Field
-                v6 = not v0
-                enable_side_effects v6
-                v7 = array_get v4, index v1 -> Field
-                return
-            }
-            ";
+        acir(inline) fn main f0 {
+          b0(v0: u1, v1: u64):
+            enable_side_effects v0
+            v4 = make_array [Field 0, Field 1] : [Field; 2]
+            v5 = array_get v4, index v1 -> Field
+            v6 = not v0
+            enable_side_effects v6
+            v7 = array_get v4, index v1 -> Field
+            return
+        }
+        ";
         let ssa = Ssa::from_str(src).unwrap();
 
         // Expected output is unchanged
@@ -1096,7 +1085,6 @@ mod test {
         //     v5 = call keccakf1600(v1)
         //     v6 = call keccakf1600(v2)
         // }
-        //
         // Here we're checking a situation where two identical arrays are being initialized twice and being assigned separate `ValueId`s.
         // This would result in otherwise identical instructions not being deduplicated.
         let main_id = Id::test_new(0);
diff --git a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
index e6ff8b31594..c8dd0e3c5a3 100644
--- a/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
+++ b/compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
@@ -329,7 +329,6 @@ impl<'f> Context<'f> {
         // If this is not a separate variable, clippy gets confused and says the to_vec is
         // unnecessary, when removing it actually causes an aliasing/mutability error.
         let instructions = self.inserter.function.dfg[block].instructions().to_vec();
-
         for instruction in instructions.iter() {
             if self.is_no_predicate(no_predicates, instruction) {
                 // disable side effect for no_predicate functions
@@ -637,10 +636,6 @@ impl<'f> Context<'f> {
 
     /// If we are currently in a branch, we need to modify constrain instructions
     /// to multiply them by the branch's condition (see optimization #1 in the module comment).
-    ///
-    /// `previous_allocate_result` should only be set to the result of an allocate instruction
-    /// if that instruction was the instruction immediately previous to this one - if there are
-    /// any instructions in between it should be None.
     fn handle_instruction_side_effects(
         &mut self,
         instruction: Instruction,
@@ -1282,6 +1277,7 @@ mod test {
     fn should_not_merge_incorrectly_to_false() {
         // Regression test for #1792
         // Tests that it does not simplify a true constraint an always-false constraint
+
         let src = "
         acir(inline) fn main f0 {
           b0(v0: [u8; 2]):

From eec5970658157704dac5c41e6d61b2aa652ce996 Mon Sep 17 00:00:00 2001
From: Ary Borenszweig <asterite@gmail.com>
Date: Fri, 29 Nov 2024 10:04:58 -0300
Subject: [PATCH 6/6] fix: correct types returned by constant EC operations
 simplified within SSA (#6652)

Co-authored-by: TomAFrench <tom@tomfren.ch>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
---
 .../src/ssa/ir/instruction/call.rs            | 34 ++++++++++++-------
 .../src/ssa/ir/instruction/call/blackbox.rs   |  4 +--
 .../Nargo.toml                                |  7 ++++
 .../src/main.nr                               | 15 ++++++++
 4 files changed, 46 insertions(+), 14 deletions(-)
 create mode 100644 test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml
 create mode 100644 test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr

diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
index 4be37b3c626..67222d06ea8 100644
--- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
+++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs
@@ -45,17 +45,17 @@ pub(super) fn simplify_call(
         _ => return SimplifyResult::None,
     };
 
+    let return_type = ctrl_typevars.and_then(|return_types| return_types.first().cloned());
+
     let constant_args: Option<Vec<_>> =
         arguments.iter().map(|value_id| dfg.get_numeric_constant(*value_id)).collect();
 
-    match intrinsic {
+    let simplified_result = match intrinsic {
         Intrinsic::ToBits(endian) => {
             // TODO: simplify to a range constraint if `limb_count == 1`
-            if let (Some(constant_args), Some(return_type)) =
-                (constant_args, ctrl_typevars.map(|return_types| return_types.first().cloned()))
-            {
+            if let (Some(constant_args), Some(return_type)) = (constant_args, return_type.clone()) {
                 let field = constant_args[0];
-                let limb_count = if let Some(Type::Array(_, array_len)) = return_type {
+                let limb_count = if let Type::Array(_, array_len) = return_type {
                     array_len as u32
                 } else {
                     unreachable!("ICE: Intrinsic::ToRadix return type must be array")
@@ -67,12 +67,10 @@ pub(super) fn simplify_call(
         }
         Intrinsic::ToRadix(endian) => {
             // TODO: simplify to a range constraint if `limb_count == 1`
-            if let (Some(constant_args), Some(return_type)) =
-                (constant_args, ctrl_typevars.map(|return_types| return_types.first().cloned()))
-            {
+            if let (Some(constant_args), Some(return_type)) = (constant_args, return_type.clone()) {
                 let field = constant_args[0];
                 let radix = constant_args[1].to_u128() as u32;
-                let limb_count = if let Some(Type::Array(_, array_len)) = return_type {
+                let limb_count = if let Type::Array(_, array_len) = return_type {
                     array_len as u32
                 } else {
                     unreachable!("ICE: Intrinsic::ToRadix return type must be array")
@@ -330,7 +328,7 @@ pub(super) fn simplify_call(
         }
         Intrinsic::FromField => {
             let incoming_type = Type::field();
-            let target_type = ctrl_typevars.unwrap().remove(0);
+            let target_type = return_type.clone().unwrap();
 
             let truncate = Instruction::Truncate {
                 value: arguments[0],
@@ -352,8 +350,8 @@ pub(super) fn simplify_call(
         Intrinsic::AsWitness => SimplifyResult::None,
         Intrinsic::IsUnconstrained => SimplifyResult::None,
         Intrinsic::DerivePedersenGenerators => {
-            if let Some(Type::Array(_, len)) = ctrl_typevars.unwrap().first() {
-                simplify_derive_generators(dfg, arguments, *len as u32, block, call_stack)
+            if let Some(Type::Array(_, len)) = return_type.clone() {
+                simplify_derive_generators(dfg, arguments, len as u32, block, call_stack)
             } else {
                 unreachable!("Derive Pedersen Generators must return an array");
             }
@@ -370,7 +368,19 @@ pub(super) fn simplify_call(
         }
         Intrinsic::ArrayRefCount => SimplifyResult::None,
         Intrinsic::SliceRefCount => SimplifyResult::None,
+    };
+
+    if let (Some(expected_types), SimplifyResult::SimplifiedTo(result)) =
+        (return_type, &simplified_result)
+    {
+        assert_eq!(
+            dfg.type_of_value(*result),
+            expected_types,
+            "Simplification should not alter return type"
+        );
     }
+
+    simplified_result
 }
 
 /// Slices have a tuple structure (slice length, slice contents) to enable logic
diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs
index 4f2a31e2fb0..301b75e0bd4 100644
--- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs
+++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs
@@ -48,7 +48,7 @@ pub(super) fn simplify_ec_add(
 
             let result_x = dfg.make_constant(result_x, Type::field());
             let result_y = dfg.make_constant(result_y, Type::field());
-            let result_is_infinity = dfg.make_constant(result_is_infinity, Type::bool());
+            let result_is_infinity = dfg.make_constant(result_is_infinity, Type::field());
 
             let typ = Type::Array(Arc::new(vec![Type::field()]), 3);
 
@@ -107,7 +107,7 @@ pub(super) fn simplify_msm(
 
             let result_x = dfg.make_constant(result_x, Type::field());
             let result_y = dfg.make_constant(result_y, Type::field());
-            let result_is_infinity = dfg.make_constant(result_is_infinity, Type::bool());
+            let result_is_infinity = dfg.make_constant(result_is_infinity, Type::field());
 
             let elements = im::vector![result_x, result_y, result_is_infinity];
             let typ = Type::Array(Arc::new(vec![Type::field()]), 3);
diff --git a/test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml b/test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml
new file mode 100644
index 00000000000..ecac2dfb197
--- /dev/null
+++ b/test_programs/execution_success/inline_decompose_hint_brillig_call/Nargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "inline_decompose_hint_brillig_call"
+version = "0.1.0"
+type = "bin"
+authors = [""]
+
+[dependencies]
diff --git a/test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr b/test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr
new file mode 100644
index 00000000000..e500f0f976d
--- /dev/null
+++ b/test_programs/execution_success/inline_decompose_hint_brillig_call/src/main.nr
@@ -0,0 +1,15 @@
+use std::embedded_curve_ops::{EmbeddedCurvePoint, EmbeddedCurveScalar, fixed_base_scalar_mul};
+
+fn main() -> pub Field {
+    let pre_address = 0x23d95e303879a5d0bbef78ecbc335e559da37431f6dcd11da54ed375c2846813;
+    let (a, b) = std::field::bn254::decompose(pre_address);
+    let curve = EmbeddedCurveScalar { lo: a, hi: b };
+    let key = fixed_base_scalar_mul(curve);
+    let point = EmbeddedCurvePoint {
+        x: 0x111223493147f6785514b1c195bb37a2589f22a6596d30bb2bb145fdc9ca8f1e,
+        y: 0x273bbffd678edce8fe30e0deafc4f66d58357c06fd4a820285294b9746c3be95,
+        is_infinite: false,
+    };
+    let address_point = key.add(point);
+    address_point.x
+}