From 1111465551557ed9e97e4b43d6eccc4b5896a39f Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 4 Jan 2024 16:29:32 +0000 Subject: [PATCH] fix: preserve brillig entrypoint functions without arguments (#3951) # Description ## Problem\* Resolves ## 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. --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 16 ++++++++++++---- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 5 ++++- .../brillig_set_slice_of_slice/Nargo.toml | 0 .../brillig_set_slice_of_slice/src/main.nr | 0 .../brillig_to_bits/Nargo.toml | 0 .../brillig_to_bits/src/main.nr | 0 .../unconstrained_empty/Nargo.toml | 0 .../unconstrained_empty/src/main.nr | 0 8 files changed, 16 insertions(+), 5 deletions(-) rename test_programs/{compile_success_empty => execution_success}/brillig_set_slice_of_slice/Nargo.toml (100%) rename test_programs/{compile_success_empty => execution_success}/brillig_set_slice_of_slice/src/main.nr (100%) rename test_programs/{compile_success_empty => execution_success}/brillig_to_bits/Nargo.toml (100%) rename test_programs/{compile_success_empty => execution_success}/brillig_to_bits/src/main.nr (100%) rename test_programs/{compile_success_empty => execution_success}/unconstrained_empty/Nargo.toml (100%) rename test_programs/{compile_success_empty => execution_success}/unconstrained_empty/src/main.nr (100%) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 3bc99b4d054..ddafc0bb570 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -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); @@ -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"); @@ -1310,6 +1312,7 @@ impl AcirContext { generated_brillig: GeneratedBrillig, inputs: Vec, outputs: Vec, + attempt_execution: bool, ) -> Result, InternalError> { let b_inputs = try_vecmap(inputs, |i| match i { AcirValue::Var(var, _) => Ok(BrilligInputs::Single(self.var_to_expression(var)?)), @@ -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. diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 33f00796c9d..edf0461430f 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -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() @@ -489,7 +492,7 @@ impl Context { let outputs: Vec = 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"); diff --git a/test_programs/compile_success_empty/brillig_set_slice_of_slice/Nargo.toml b/test_programs/execution_success/brillig_set_slice_of_slice/Nargo.toml similarity index 100% rename from test_programs/compile_success_empty/brillig_set_slice_of_slice/Nargo.toml rename to test_programs/execution_success/brillig_set_slice_of_slice/Nargo.toml diff --git a/test_programs/compile_success_empty/brillig_set_slice_of_slice/src/main.nr b/test_programs/execution_success/brillig_set_slice_of_slice/src/main.nr similarity index 100% rename from test_programs/compile_success_empty/brillig_set_slice_of_slice/src/main.nr rename to test_programs/execution_success/brillig_set_slice_of_slice/src/main.nr diff --git a/test_programs/compile_success_empty/brillig_to_bits/Nargo.toml b/test_programs/execution_success/brillig_to_bits/Nargo.toml similarity index 100% rename from test_programs/compile_success_empty/brillig_to_bits/Nargo.toml rename to test_programs/execution_success/brillig_to_bits/Nargo.toml diff --git a/test_programs/compile_success_empty/brillig_to_bits/src/main.nr b/test_programs/execution_success/brillig_to_bits/src/main.nr similarity index 100% rename from test_programs/compile_success_empty/brillig_to_bits/src/main.nr rename to test_programs/execution_success/brillig_to_bits/src/main.nr diff --git a/test_programs/compile_success_empty/unconstrained_empty/Nargo.toml b/test_programs/execution_success/unconstrained_empty/Nargo.toml similarity index 100% rename from test_programs/compile_success_empty/unconstrained_empty/Nargo.toml rename to test_programs/execution_success/unconstrained_empty/Nargo.toml diff --git a/test_programs/compile_success_empty/unconstrained_empty/src/main.nr b/test_programs/execution_success/unconstrained_empty/src/main.nr similarity index 100% rename from test_programs/compile_success_empty/unconstrained_empty/src/main.nr rename to test_programs/execution_success/unconstrained_empty/src/main.nr