Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: preserve brillig entrypoint functions without arguments #3951

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
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
Loading