diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 4ec39de655d..c4c811cb900 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -242,10 +242,8 @@ impl<'brillig> Context<'brillig> { function.dfg.make_constant(FieldElement::one(), NumericType::bool()); for instruction_id in instructions { - if !self.values_to_replace.is_empty() { - let instruction = &mut function.dfg[instruction_id]; - instruction.replace_values(&self.values_to_replace); - } + let instruction = &mut function.dfg[instruction_id]; + instruction.replace_values(&self.values_to_replace); self.fold_constants_into_instruction( function, @@ -256,11 +254,18 @@ impl<'brillig> Context<'brillig> { ); } + // Map the block terminator, resolving any values in the terminator with the + // internal value mapping generated by this pass. function.dfg.replace_values_in_block_terminator(block_id, &self.values_to_replace); function.dfg.data_bus.replace_values(&self.values_to_replace); // Map a terminator in place, replacing any ValueId in the terminator with the // resolved version of that value id from the simplification cache's internal value mapping. + // We need this in addition to the value replacement above in order to take advantage + // of constraints that may have advised simplifications. + // The value mapping (`self.values_to_replace`) only maps old instruction results to new instruction results. + // However, constraints do not have "results" like other instructions, thus are not included in `self.values_to_replace`. + // To take advantage of constraint simplification we need to still resolve its cache. let mut terminator = function.dfg[block_id].take_terminator(); terminator.map_values_mut(|value| { Self::resolve_cache( @@ -271,6 +276,14 @@ impl<'brillig> Context<'brillig> { ) }); function.dfg[block_id].set_terminator(terminator); + function.dfg.data_bus.map_values_mut(|value| { + Self::resolve_cache( + block_id, + dom, + self.get_constraint_map(side_effects_enabled_var), + value, + ) + }); self.block_queue.extend(function.dfg[block_id].successors()); } @@ -845,7 +858,7 @@ fn has_side_effects(instruction: &Instruction, dfg: &DataFlowGraph) -> bool { // These can fail. Constrain(..) | ConstrainNotEqual(..) | RangeCheck { .. } => true, - // This should never be side-effectful + // This should never be side-effectual MakeArray { .. } | Noop => false, // Some binary math can overflow or underflow @@ -1946,4 +1959,62 @@ mod test { } "); } + + #[test] + fn constant_fold_terminator_argument_from_constrain() { + // The only instructions advising simplifications for v0 are + // constrain instructions. We want to make sure that those simplifications + // are still used for any terminator arguments. + let src = " + brillig(inline) predicate_pure fn main f0 { + b0(v0: Field, v1: Field): + v5 = eq v0, Field 1 + constrain v0 == Field 1 + v7 = eq v1, Field 0 + constrain v1 == Field 0 + v8 = truncate v0 to 32 bits, max_bit_size: 254 + v9 = cast v8 as u32 + v11 = eq v9, u32 0 + jmpif v11 then: b1, else: b2 + b1(): + v13 = add v0, Field 1 + jmp b3(v0, v13) + b2(): + v12 = add v0, Field 1 + jmp b3(v12, v0) + b3(v2: Field, v3: Field): + v14 = add v0, Field 1 + v15 = eq v2, v14 + constrain v2 == v14 + v16 = eq v3, v0 + constrain v3 == v0 + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + let ssa = ssa.fold_constants_using_constraints(); + + // The terminators of b1 and b2 should now have constant arguments + assert_ssa_snapshot!(ssa, @r" + brillig(inline) predicate_pure fn main f0 { + b0(v0: Field, v1: Field): + v5 = eq v0, Field 1 + constrain v0 == Field 1 + v7 = eq v1, Field 0 + constrain v1 == Field 0 + jmpif u1 0 then: b1, else: b2 + b1(): + jmp b3(Field 1, Field 2) + b2(): + jmp b3(Field 2, Field 1) + b3(v2: Field, v3: Field): + v10 = eq v2, Field 2 + constrain v2 == Field 2 + v11 = eq v3, Field 1 + constrain v3 == Field 1 + return + } + "); + } }