From 6f72c0af6f953b828f6c6c581ec39915df555d97 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 1 Oct 2025 16:19:57 +0200 Subject: [PATCH 1/3] hoist and then deduplicate --- .../src/ssa/opt/constant_folding/mod.rs | 68 +++++++++++++++++-- 1 file changed, 62 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs index ab4a2895963..7192baad9de 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs @@ -279,11 +279,16 @@ impl Context { return; } 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. + // Insert a copy of the instruction in the common dominator. // When constant folding is run a second time later on, it'll catch - // that the previous instance can be deduplicated to this instance. - block = dominator; + // that the previous instances can be deduplicated to this instance. + let call_stack = dfg.get_instruction_call_stack_id(id); + dfg.insert_instruction_and_results( + instruction.clone(), + dominator, + None, + call_stack, + ); } } }; @@ -938,8 +943,9 @@ mod test { // v4 has been hoisted, although: // - v5 has not yet been removed since it was encountered earlier in the program - // - v8 hasn't been recognized as a duplicate of v6 yet since they still reference v4 and - // v5 respectively + // - v8 has not yet been removed since v4 was not in the cache + // - v9 hasn't been recognized as a duplicate of v6 yet since they still reference v5 and + // v8 respectively assert_ssa_snapshot!(ssa, @r" brillig(inline) fn main f0 { b0(v0: u32): @@ -953,6 +959,56 @@ mod test { jmp b2() b2(): jmpif v2 then: b3, else: b4 + b3(): + v8 = shl v0, u32 1 + v9 = lt v0, v8 + constrain v9 == u1 1 + jmp b4() + b4(): + return + } + "); + + // src is equal to the previous output + let src = " + brillig(inline) fn main f0 { + b0(v0: u32): + v2 = lt u32 1000, v0 + v4 = shl v0, u32 1 + jmpif v2 then: b1, else: b2 + b1(): + v5 = shl v0, u32 1 + v6 = lt v0, v5 + constrain v6 == u1 1 + jmp b2() + b2(): + jmpif v2 then: b3, else: b4 + b3(): + v8 = shl v0, u32 1 + v9 = lt v0, v8 + constrain v9 == u1 1 + jmp b4() + b4(): + return + } + "; + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.fold_constants_using_constraints(); + // Another round of folding has fully deduplicated the shl + // v6 and v8 are now ready to be deduplicated by v5 + assert_ssa_snapshot!(ssa, @r" + brillig(inline) fn main f0 { + b0(v0: u32): + v2 = lt u32 1000, v0 + v4 = shl v0, u32 1 + v5 = lt v0, v4 + jmpif v2 then: b1, else: b2 + b1(): + v6 = lt v0, v4 + constrain v6 == u1 1 + jmp b2() + b2(): + jmpif v2 then: b3, else: b4 b3(): v8 = lt v0, v4 constrain v8 == u1 1 From db019f8e052e26c112f1e6e78e610e8e357b9b0c Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 2 Oct 2025 09:34:34 +0200 Subject: [PATCH 2/3] hoist and deduplicate, with ref_count --- .../src/ssa/opt/constant_folding/mod.rs | 166 ++++++++++-------- 1 file changed, 90 insertions(+), 76 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs index 7192baad9de..a022af73203 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs @@ -234,7 +234,7 @@ impl Context { &mut self, dfg: &mut DataFlowGraph, dom: &mut DominatorTree, - mut block: BasicBlockId, + block: BasicBlockId, id: InstructionId, side_effects_enabled_var: &mut ValueId, interpreter: &mut Option>, @@ -246,6 +246,7 @@ impl Context { Self::resolve_instruction(id, block, dfg, dom, constraint_simplification_mapping); let old_results = dfg.instruction_results(id).to_vec(); + let mut target_block = block; // If a copy of this instruction exists earlier in the block, then reuse the previous results. let runtime_is_brillig = dfg.runtime().is_brillig(); @@ -259,55 +260,52 @@ impl Context { // them but we still need to issue an extra inc_rc in case they're mutated afterward. // // This also applies to calls that return arrays. - if runtime_is_brillig - && matches!( - instruction, - Instruction::MakeArray { .. } | Instruction::Call { .. } - ) - { - let call_stack = dfg.get_instruction_call_stack_id(id); - for &value in cached { - let value_type = dfg.type_of_value(value); - if value_type.is_array() { - let inc_rc = Instruction::IncrementRc { value }; - dfg.insert_instruction_and_results(inc_rc, block, None, call_stack); - } - } + if runtime_is_brillig { + Self::increase_rc(id, cached, block, dfg); } self.values_to_replace.batch_insert(&old_results, cached); return; } CacheResult::NeedToHoistToCommonBlock(dominator) => { - // Insert a copy of the instruction in the common 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 - // that the previous instances can be deduplicated to this instance. - let call_stack = dfg.get_instruction_call_stack_id(id); - dfg.insert_instruction_and_results( - instruction.clone(), - dominator, - None, - call_stack, - ); + // that the previous instance can be deduplicated to this instance. + target_block = dominator; } } }; // First try to inline a call to a brillig function with all constant arguments. let new_results = if runtime_is_brillig { - Self::push_instruction(id, instruction.clone(), &old_results, block, dfg) + Self::push_instruction(id, instruction.clone(), &old_results, target_block, dfg) } else { // We only want to try to inline Brillig calls for Brillig entry points (functions called from an ACIR runtime). - try_interpret_call(&instruction, block, dfg, interpreter.as_mut()) + try_interpret_call(&instruction, target_block, dfg, interpreter.as_mut()) // Otherwise, try inserting the instruction again to apply any optimizations using the newly resolved inputs. .unwrap_or_else(|| { - Self::push_instruction(id, instruction.clone(), &old_results, block, dfg) + Self::push_instruction(id, instruction.clone(), &old_results, target_block, dfg) }) }; + // If the target_block is distinct than the original block + // that means that the current instruction is not added in the orignal block + // so it is deduplicated by the one in the target block. + // In case it refers to an array that is mutated, we need to increment + // its reference count. + if target_block != block && runtime_is_brillig { + Self::increase_rc(id, &new_results, block, dfg); + } self.values_to_replace.batch_insert(&old_results, &new_results); - self.cache_instruction(&instruction, new_results, dfg, *side_effects_enabled_var, block); + self.cache_instruction( + &instruction, + new_results, + dfg, + *side_effects_enabled_var, + target_block, + ); // If we just inserted an `Instruction::EnableSideEffectsIf`, we need to update `side_effects_enabled_var` // so that we use the correct set of constrained values in future. @@ -316,6 +314,25 @@ impl Context { }; } + fn increase_rc( + id: InstructionId, + results: &[ValueId], + block: BasicBlockId, + dfg: &mut DataFlowGraph, + ) { + let instruction = &dfg[id]; + if matches!(instruction, Instruction::MakeArray { .. } | Instruction::Call { .. }) { + let call_stack = dfg.get_instruction_call_stack_id(id); + for value in results { + let value_type = dfg.type_of_value(*value); + if value_type.is_array() { + let inc_rc = Instruction::IncrementRc { value: *value }; + dfg.insert_instruction_and_results(inc_rc, block, None, call_stack); + } + } + } + } + /// Fetches an [`Instruction`] by its [`InstructionId`] and fully resolves its inputs. fn resolve_instruction( instruction_id: InstructionId, @@ -943,9 +960,8 @@ mod test { // v4 has been hoisted, although: // - v5 has not yet been removed since it was encountered earlier in the program - // - v8 has not yet been removed since v4 was not in the cache - // - v9 hasn't been recognized as a duplicate of v6 yet since they still reference v5 and - // v8 respectively + // - v8 hasn't been recognized as a duplicate of v6 yet since they still reference v4 and + // v5 respectively assert_ssa_snapshot!(ssa, @r" brillig(inline) fn main f0 { b0(v0: u32): @@ -960,62 +976,60 @@ mod test { b2(): jmpif v2 then: b3, else: b4 b3(): - v8 = shl v0, u32 1 - v9 = lt v0, v8 - constrain v9 == u1 1 + v8 = lt v0, v4 + constrain v8 == u1 1 jmp b4() b4(): return } "); + } - // src is equal to the previous output + #[test] + fn increment_rc_on_make_array_deduplication() { let src = " - brillig(inline) fn main f0 { - b0(v0: u32): - v2 = lt u32 1000, v0 - v4 = shl v0, u32 1 - jmpif v2 then: b1, else: b2 - b1(): - v5 = shl v0, u32 1 - v6 = lt v0, v5 - constrain v6 == u1 1 - jmp b2() - b2(): - jmpif v2 then: b3, else: b4 - b3(): - v8 = shl v0, u32 1 - v9 = lt v0, v8 - constrain v9 == u1 1 - jmp b4() - b4(): - return - } + brillig(inline) fn main f0 { + b0(v0: u32): + v2 = lt u32 1000, v0 + jmpif v2 then: b1, else: b2 + b1(): + v4 = make_array [u1 0] : [u1; 1] + v5 = array_get v4, index u32 0 -> u1 + jmp b3(v5) + b2(): + v7 = make_array [u1 0] : [u1; 1] + v8 = array_get v7, index u32 0 -> u1 + jmp b3(v8) + b3(v9: u1): + constrain v9 == u1 0 + jmp b4() + b4(): + return + } "; let ssa = Ssa::from_str(src).unwrap(); let ssa = ssa.fold_constants_using_constraints(); - // Another round of folding has fully deduplicated the shl - // v6 and v8 are now ready to be deduplicated by v5 + + // v7 has been replaced by a v5, and its reference count is increased + // v6 is not yet replaced but will be in a subsequent constant folding run assert_ssa_snapshot!(ssa, @r" - brillig(inline) fn main f0 { - b0(v0: u32): - v2 = lt u32 1000, v0 - v4 = shl v0, u32 1 - v5 = lt v0, v4 - jmpif v2 then: b1, else: b2 - b1(): - v6 = lt v0, v4 - constrain v6 == u1 1 - jmp b2() - b2(): - jmpif v2 then: b3, else: b4 - b3(): - v8 = lt v0, v4 - constrain v8 == u1 1 - jmp b4() - b4(): - return - } + brillig(inline) fn main f0 { + b0(v0: u32): + v3 = lt u32 1000, v0 + v5 = make_array [u1 0] : [u1; 1] + jmpif v3 then: b1, else: b2 + b1(): + v6 = make_array [u1 0] : [u1; 1] + jmp b3(u1 0) + b2(): + inc_rc v5 + jmp b3(u1 0) + b3(v1: u1): + constrain v1 == u1 0 + jmp b4() + b4(): + return + } "); } From 6df85580f05432e3bfde3a9110b89d0c04f7cb9f Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 6 Oct 2025 19:51:26 +0200 Subject: [PATCH 3/3] update test cases --- .../src/ssa/opt/constant_folding/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs index 7c185181c4d..8ba568f8d1e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs @@ -1079,10 +1079,10 @@ mod test { v5 = make_array [u1 0] : [u1; 1] jmpif v3 then: b1, else: b2 b1(): - v6 = make_array [u1 0] : [u1; 1] + inc_rc v5 jmp b3(u1 0) b2(): - inc_rc v5 + v6 = make_array [u1 0] : [u1; 1] jmp b3(u1 0) b3(v1: u1): constrain v1 == u1 0 @@ -1336,6 +1336,7 @@ mod test { v28 = make_array b"DEF" jmpif v20 then: b1, else: b2 b1(): + inc_rc v24 jmp b3(v24) b2(): v29 = eq v19, Field 3 @@ -1365,6 +1366,7 @@ mod test { b8(v2: [u8; 3]): jmp b3(v2) b9(): + inc_rc v28 jmp b10() b10(): inc_rc v28 @@ -1393,6 +1395,7 @@ mod test { jmp b20(v28) b19(): constrain v34 == Field 5 + inc_rc v28 jmp b20(v28) b20(v6: [u8; 3]): jmp b17(v6) @@ -1461,6 +1464,7 @@ mod test { v11 = make_array [u8 0] : [u8; 1] jmpif v0 then: b3, else: b4 b3(): + inc_rc v11 jmp b5() b4(): inc_rc v11 @@ -1596,6 +1600,7 @@ mod test { jmpif v1 then: b7, else: b8 b7(): v9 = make_array [u8 0] : [u8; 1] + inc_rc v8 jmp b9() b8(): inc_rc v8 @@ -1607,6 +1612,7 @@ mod test { jmpif v1 then: b11, else: b12 b11(): inc_rc v3 + inc_rc v5 jmp b13() b12(): inc_rc v5 @@ -1620,6 +1626,7 @@ mod test { store v3 at v6 jmp b16() b16(): + inc_rc v3 v10 = allocate -> &mut [u8; 1] store v3 at v10 jmp b17() @@ -1628,6 +1635,7 @@ mod test { v12 = make_array [u8 3] : [u8; 1] jmpif v1 then: b18, else: b19 b18(): + inc_rc v12 jmp b20() b19(): inc_rc v12