Skip to content

Commit

Permalink
fix: preserve brillig entrypoint functions without arguments (#3951)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

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

## Summary\*

This PR turns off the optimistic execution of brillig functions for
which all arguments are known at compile time when that brillig function
is the program entrypoint.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored Jan 4, 2024
1 parent abf751a commit 1111465
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 5 deletions.
16 changes: 12 additions & 4 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ impl AcirContext {
inverse_code,
vec![AcirValue::Var(var, AcirType::field())],
vec![AcirType::field()],
true,
)?;
let inverted_var = Self::expect_one_var(results);

Expand Down Expand Up @@ -708,6 +709,7 @@ impl AcirContext {
AcirValue::Var(rhs, AcirType::unsigned(bit_size)),
],
vec![AcirType::unsigned(max_q_bits), AcirType::unsigned(max_rhs_bits)],
true,
)?
.try_into()
.expect("quotient only returns two values");
Expand Down Expand Up @@ -1310,6 +1312,7 @@ impl AcirContext {
generated_brillig: GeneratedBrillig,
inputs: Vec<AcirValue>,
outputs: Vec<AcirType>,
attempt_execution: bool,
) -> Result<Vec<AcirValue>, InternalError> {
let b_inputs = try_vecmap(inputs, |i| match i {
AcirValue::Var(var, _) => Ok(BrilligInputs::Single(self.var_to_expression(var)?)),
Expand All @@ -1329,10 +1332,15 @@ impl AcirContext {

// Optimistically try executing the brillig now, if we can complete execution they just return the results.
// This is a temporary measure pending SSA optimizations being applied to Brillig which would remove constant-input opcodes (See #2066)
if let Some(brillig_outputs) =
self.execute_brillig(&generated_brillig.byte_code, &b_inputs, &outputs)
{
return Ok(brillig_outputs);
//
// We do _not_ want to do this in the situation where the `main` function is unconstrained, as if execution succeeds
// the entire program will be replaced with witness constraints to its outputs.
if attempt_execution {
if let Some(brillig_outputs) =
self.execute_brillig(&generated_brillig.byte_code, &b_inputs, &outputs)
{
return Ok(brillig_outputs);
}
}

// Otherwise we must generate ACIR for it and execute at runtime.
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,14 @@ impl Context {

let code = self.gen_brillig_for(main_func, &brillig)?;

// We specifically do not attempt execution of the brillig code being generated as this can result in it being
// replaced with constraints on witnesses to the program outputs.
let output_values = self.acir_context.brillig(
self.current_side_effects_enabled_var,
code,
inputs,
outputs,
false,
)?;
let output_vars: Vec<_> = output_values
.iter()
Expand Down Expand Up @@ -489,7 +492,7 @@ impl Context {

let outputs: Vec<AcirType> = vecmap(result_ids, |result_id| dfg.type_of_value(*result_id).into());

let output_values = self.acir_context.brillig(self.current_side_effects_enabled_var, code, inputs, outputs)?;
let output_values = self.acir_context.brillig(self.current_side_effects_enabled_var, code, inputs, outputs, true)?;

// Compiler sanity check
assert_eq!(result_ids.len(), output_values.len(), "ICE: The number of Brillig output values should match the result ids in SSA");
Expand Down

0 comments on commit 1111465

Please sign in to comment.