Skip to content

Commit c9a8bf8

Browse files
authored
fix: only cache ArraySet during constant folding if using constraint … (#11110)
1 parent ea2c41b commit c9a8bf8

File tree

1 file changed

+54
-30
lines changed
  • compiler/noirc_evaluator/src/ssa/opt/constant_folding

1 file changed

+54
-30
lines changed

compiler/noirc_evaluator/src/ssa/opt/constant_folding/mod.rs

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -464,37 +464,45 @@ impl Context {
464464
block: BasicBlockId,
465465
) {
466466
if self.use_constraint_info {
467-
// If the instruction was a constraint, then create a link between the two `ValueId`s
468-
// to map from the more complex to the simpler value.
469-
if let Instruction::Constrain(lhs, rhs, _) = instruction {
470-
// These `ValueId`s should be fully resolved now.
471-
self.constraint_simplification_mappings.cache(
472-
dfg,
473-
side_effects_enabled_var,
474-
block,
475-
*lhs,
476-
*rhs,
477-
);
478-
}
479-
}
480-
481-
// If we have an array get whose value is from an array set on the same array at the same index,
482-
// we can simplify that array get to the value of the previous array set.
483-
//
484-
// For example:
485-
// v3 = array_set v0, index v1, value v2
486-
// v4 = array_get v3, index v1 -> Field
487-
//
488-
// We know that `v4` can be simplified to `v2`.
489-
// Thus, even if the index is dynamic (meaning the array get would have side effects),
490-
// we can simplify the operation when we take into account the predicate.
491-
if let Instruction::ArraySet { index, value, .. } = instruction {
492-
let predicate = self.use_constraint_info.then_some(side_effects_enabled_var);
493-
494-
let array_get = Instruction::ArrayGet { array: instruction_results[0], index: *index };
467+
match instruction {
468+
// If the instruction was a constraint, then create a link between the two `ValueId`s
469+
// to map from the more complex to the simpler value.
470+
Instruction::Constrain(lhs, rhs, _) => {
471+
// These `ValueId`s should be fully resolved now.
472+
self.constraint_simplification_mappings.cache(
473+
dfg,
474+
side_effects_enabled_var,
475+
block,
476+
*lhs,
477+
*rhs,
478+
);
479+
}
495480

496-
// If we encounter an array_get for this address, we know what the result will be.
497-
self.cached_instruction_results.cache(dom, array_get, predicate, block, vec![*value]);
481+
// If we have an array get whose value is from an array set on the same array at the same index,
482+
// we can simplify that array get to the value of the previous array set.
483+
//
484+
// For example:
485+
// v3 = array_set v0, index v1, value v2
486+
// v4 = array_get v3, index v1 -> Field
487+
//
488+
// We know that `v4` can be simplified to `v2`.
489+
// Thus, even if the index is dynamic (meaning the array get would have side effects),
490+
// we can simplify the operation when we take into account the predicate.
491+
Instruction::ArraySet { index, value, .. } => {
492+
let array_get =
493+
Instruction::ArrayGet { array: instruction_results[0], index: *index };
494+
495+
// If we encounter an array_get for this address, we know what the result will be.
496+
self.cached_instruction_results.cache(
497+
dom,
498+
array_get,
499+
Some(side_effects_enabled_var),
500+
block,
501+
vec![*value],
502+
);
503+
}
504+
_ => (),
505+
}
498506
}
499507

500508
self.cached_instruction_results
@@ -2477,4 +2485,20 @@ mod tests {
24772485

24782486
assert!(execution_result.is_ok());
24792487
}
2488+
2489+
#[test]
2490+
fn bug_array_get_from_array_set_ignores_predicate_without_constraint_info() {
2491+
let src = "
2492+
acir(inline) fn main f0 {
2493+
b0(v0: [Field; 3], v1: u32, v2: Field):
2494+
enable_side_effects u1 0
2495+
v4 = array_set v0, index v1, value v2
2496+
enable_side_effects u1 1
2497+
v6 = array_get v4, index v1 -> Field
2498+
return v6
2499+
}
2500+
";
2501+
2502+
assert_ssa_does_not_change(src, |ssa| ssa.fold_constants(MIN_ITER));
2503+
}
24802504
}

0 commit comments

Comments
 (0)