Skip to content

Commit

Permalink
fix: discard ref counts during unrolling (#4923)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #4709 

## Summary\*
The issue is due to reference count instructions which blow-up during
unrolling. Since these instructions are only for Brillig and since loops
are not unrolled in Brillig, I simply discard them during unrolling.


## Additional Context
I did not get an OOM on the mainframe, but the compilation lasted many
hours and did not complete (I finally cancelled it). With the fix the
execution is instantaneous, whether the code is constrained or
unconstrained.


## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [ ] I have tested the changes locally.
- [ ] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
3 people authored Apr 26, 2024
1 parent c4b0369 commit 91062db
Show file tree
Hide file tree
Showing 5 changed files with 290 additions and 3 deletions.
13 changes: 10 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/unrolling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
//!
//! Note that this pass also often creates superfluous jmp instructions in the
//! program that will need to be removed by a later simplify cfg pass.
//! Note also that unrolling is skipped for Brillig runtime and as a result
//! we remove reference count instructions because they are only used by Brillig bytecode
use std::collections::HashSet;

use crate::{
Expand All @@ -24,7 +26,7 @@ use crate::{
dom::DominatorTree,
function::{Function, RuntimeType},
function_inserter::FunctionInserter,
instruction::TerminatorInstruction,
instruction::{Instruction, TerminatorInstruction},
post_order::PostOrder,
value::ValueId,
},
Expand Down Expand Up @@ -465,9 +467,14 @@ impl<'f> LoopIteration<'f> {
// instances of the induction variable or any values that were changed as a result
// of the new induction variable value.
for instruction in instructions {
self.inserter.push_instruction(instruction, self.insert_block);
// Skip reference count instructions since they are only used for brillig, and brillig code is not unrolled
if !matches!(
self.dfg()[instruction],
Instruction::IncrementRc { .. } | Instruction::DecrementRc { .. }
) {
self.inserter.push_instruction(instruction, self.insert_block);
}
}

let mut terminator = self.dfg()[self.source_block]
.unwrap_terminator()
.clone()
Expand Down
6 changes: 6 additions & 0 deletions test_programs/execution_success/regression_4709/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "regression_4709"
type = "bin"
authors = [""]

[dependencies]
2 changes: 2 additions & 0 deletions test_programs/execution_success/regression_4709/Prover.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
x = 1
y = 2
Loading

0 comments on commit 91062db

Please sign in to comment.