Skip to content

Commit

Permalink
chore: Toggle underconstrained check (#5724)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

No issue as discovered when experimenting with
https://github.com/iAmMichaelConnor/blob-lib/tree/domain-size-4096.

When compiling the circuit on the branch linked above using Noir master
this was the result of timing `nargo compile`:
```
nargo compile --force --benchmark-codegen --silence-warnings  383.30s user 148.34s system 131% cpu 6:44.03 total
```
with under constrained check taking 200ms (about half the compilation
time).

The check returned no bugs. If the developer wants to iterate more
quickly and knows they are not changing unconstrained code or simply
wishes to check for under constrained bugs later they should have the
ability.

## Summary\*

This PR simply adds a flag `--skip-underconstrained-check` that skips
calling `ssa.check_for_underconstrained_values()`.

The blob-lib linked above compiled in half the time with the
underconstrained check off:
```
nargo compile --force --skip-underconstrained-check --benchmark-codegen  192.15s user 295.27s system 220% cpu 3:41.33 total
``` 

## Additional Context



## Documentation\*

Check one:
- [ ] 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.
  • Loading branch information
vezenovm authored Aug 22, 2024
1 parent 19ffa20 commit eb54614
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 1 deletion.
7 changes: 7 additions & 0 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ pub struct CompileOptions {
/// Temporary flag to enable the experimental arithmetic generics feature
#[arg(long, hide = true)]
pub arithmetic_generics: bool,

/// Flag to turn off the compiler check for under constrained values.
/// Warning: This can improve compilation speed but can also lead to correctness errors.
/// This check should always be run on production code.
#[arg(long)]
pub skip_underconstrained_check: bool,
}

pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {
Expand Down Expand Up @@ -574,6 +580,7 @@ pub fn compile_no_check(
ExpressionWidth::default()
},
emit_ssa: if options.emit_ssa { Some(context.package_build_path.clone()) } else { None },
skip_underconstrained_check: options.skip_underconstrained_check,
};

let SsaProgramArtifact { program, debug, warnings, names, brillig_names, error_types, .. } =
Expand Down
11 changes: 10 additions & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ pub struct SsaEvaluatorOptions {

/// Dump the unoptimized SSA to the supplied path if it exists
pub emit_ssa: Option<PathBuf>,

/// Skip the check for under constrained values
pub skip_underconstrained_check: bool,
}

pub(crate) struct ArtifactsAndWarnings(Artifacts, Vec<SsaReport>);
Expand Down Expand Up @@ -117,7 +120,13 @@ pub(crate) fn optimize_into_acir(
.run_pass(Ssa::array_set_optimization, "After Array Set Optimizations:")
.finish();

let ssa_level_warnings = ssa.check_for_underconstrained_values();
let ssa_level_warnings = if options.skip_underconstrained_check {
vec![]
} else {
time("After Check for Underconstrained Values", options.print_codegen_timings, || {
ssa.check_for_underconstrained_values()
})
};
let brillig = time("SSA to Brillig", options.print_codegen_timings, || {
ssa.to_brillig(options.enable_brillig_logging)
});
Expand Down
1 change: 1 addition & 0 deletions cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@
"typevar",
"typevars",
"udiv",
"underconstrained",
"uninstantiated",
"unnormalized",
"unoptimized",
Expand Down

0 comments on commit eb54614

Please sign in to comment.