Skip to content

Commit d278c56

Browse files
committed
Fix bug by restriting view
1 parent f27b1d2 commit d278c56

File tree

1 file changed

+36
-16
lines changed

1 file changed

+36
-16
lines changed

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

+36-16
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ impl Function {
146146
use_constraint_info: bool,
147147
brillig_info: Option<BrilligInfo>,
148148
) {
149-
let mut context = Context::new(self, use_constraint_info, brillig_info);
149+
let mut context = Context::new(use_constraint_info, brillig_info);
150150
context.block_queue.push_back(self.entry_block());
151151

152152
while let Some(block) = context.block_queue.pop_front() {
@@ -181,8 +181,6 @@ struct Context<'a> {
181181

182182
// Cache of instructions without any side-effects along with their outputs.
183183
cached_instruction_results: InstructionResultCache,
184-
185-
dom: DominatorTree,
186184
}
187185

188186
#[derive(Copy, Clone)]
@@ -225,6 +223,14 @@ impl SimplificationCache {
225223
}
226224
}
227225
}
226+
227+
fn get(&self, block: BasicBlockId, dom: &mut DominatorTree) -> Option<ValueId> {
228+
if self.blocks.iter().any(|b| dom.dominates(*b, block)) {
229+
Some(self.simplified)
230+
} else {
231+
None
232+
}
233+
}
228234
}
229235

230236
/// HashMap from Instruction to a simplified expression that it can be replaced with based on
@@ -251,19 +257,14 @@ struct ResultCache {
251257
}
252258

253259
impl<'brillig> Context<'brillig> {
254-
fn new(
255-
function: &Function,
256-
use_constraint_info: bool,
257-
brillig_info: Option<BrilligInfo<'brillig>>,
258-
) -> Self {
260+
fn new(use_constraint_info: bool, brillig_info: Option<BrilligInfo<'brillig>>) -> Self {
259261
Self {
260262
use_constraint_info,
261263
brillig_info,
262264
visited_blocks: Default::default(),
263265
block_queue: Default::default(),
264266
constraint_simplification_mappings: Default::default(),
265267
cached_instruction_results: Default::default(),
266-
dom: DominatorTree::with_function(function),
267268
}
268269
}
269270

@@ -273,9 +274,12 @@ impl<'brillig> Context<'brillig> {
273274
let mut side_effects_enabled_var =
274275
function.dfg.make_constant(FieldElement::one(), Type::bool());
275276

277+
let mut dom = DominatorTree::with_function(&function);
278+
276279
for instruction_id in instructions {
277280
self.fold_constants_into_instruction(
278281
&mut function.dfg,
282+
&mut dom,
279283
block,
280284
instruction_id,
281285
&mut side_effects_enabled_var,
@@ -287,17 +291,21 @@ impl<'brillig> Context<'brillig> {
287291
fn fold_constants_into_instruction(
288292
&mut self,
289293
dfg: &mut DataFlowGraph,
294+
dom: &mut DominatorTree,
290295
mut block: BasicBlockId,
291296
id: InstructionId,
292297
side_effects_enabled_var: &mut ValueId,
293298
) {
294299
let constraint_simplification_mapping = self.get_constraint_map(*side_effects_enabled_var);
295-
let instruction = Self::resolve_instruction(id, dfg, constraint_simplification_mapping);
300+
301+
let instruction =
302+
Self::resolve_instruction(id, block, dfg, dom, constraint_simplification_mapping);
303+
296304
let old_results = dfg.instruction_results(id).to_vec();
297305

298306
// If a copy of this instruction exists earlier in the block, then reuse the previous results.
299307
if let Some(cache_result) =
300-
self.get_cached(dfg, &instruction, *side_effects_enabled_var, block)
308+
self.get_cached(dfg, dom, &instruction, *side_effects_enabled_var, block)
301309
{
302310
match cache_result {
303311
CacheResult::Cached(cached) => {
@@ -354,7 +362,9 @@ impl<'brillig> Context<'brillig> {
354362
/// Fetches an [`Instruction`] by its [`InstructionId`] and fully resolves its inputs.
355363
fn resolve_instruction(
356364
instruction_id: InstructionId,
365+
block: BasicBlockId,
357366
dfg: &DataFlowGraph,
367+
dom: &mut DominatorTree,
358368
constraint_simplification_mapping: &HashMap<ValueId, SimplificationCache>,
359369
) -> Instruction {
360370
let instruction = dfg[instruction_id].clone();
@@ -365,20 +375,29 @@ impl<'brillig> Context<'brillig> {
365375
// This allows us to reach a stable final `ValueId` for each instruction input as we add more
366376
// constraints to the cache.
367377
fn resolve_cache(
378+
block: BasicBlockId,
368379
dfg: &DataFlowGraph,
380+
dom: &mut DominatorTree,
369381
cache: &HashMap<ValueId, SimplificationCache>,
370382
value_id: ValueId,
371383
) -> ValueId {
372384
let resolved_id = dfg.resolve(value_id);
373385
match cache.get(&resolved_id) {
374-
Some(cached_value) => resolve_cache(dfg, cache, cached_value.simplified),
386+
Some(simplification_cache) => {
387+
if let Some(simplified) = simplification_cache.get(block, dom) {
388+
resolve_cache(block, dfg, dom, cache, simplified)
389+
} else {
390+
resolved_id
391+
}
392+
}
375393
None => resolved_id,
376394
}
377395
}
378396

379397
// Resolve any inputs to ensure that we're comparing like-for-like instructions.
380-
instruction
381-
.map_values(|value_id| resolve_cache(dfg, constraint_simplification_mapping, value_id))
398+
instruction.map_values(|value_id| {
399+
resolve_cache(block, dfg, dom, constraint_simplification_mapping, value_id)
400+
})
382401
}
383402

384403
/// Pushes a new [`Instruction`] into the [`DataFlowGraph`] which applies any optimizations
@@ -468,8 +487,9 @@ impl<'brillig> Context<'brillig> {
468487
}
469488

470489
fn get_cached(
471-
&mut self,
490+
&self,
472491
dfg: &DataFlowGraph,
492+
dom: &mut DominatorTree,
473493
instruction: &Instruction,
474494
side_effects_enabled_var: ValueId,
475495
block: BasicBlockId,
@@ -479,7 +499,7 @@ impl<'brillig> Context<'brillig> {
479499
let predicate = self.use_constraint_info && instruction.requires_acir_gen_predicate(dfg);
480500
let predicate = predicate.then_some(side_effects_enabled_var);
481501

482-
results_for_instruction.get(&predicate)?.get(block, &mut self.dom)
502+
results_for_instruction.get(&predicate)?.get(block, dom)
483503
}
484504

485505
/// Checks if the given instruction is a call to a brillig function with all constant arguments.

0 commit comments

Comments
 (0)