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

feat(acir_gen): Use predicates with MemoryOps #2400

Closed
wants to merge 12 commits into from
24 changes: 8 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,8 @@ tower = "0.4"
url = "2.2.0"
wasm-bindgen = { version = "=0.2.86", features = ["serde-serialize"] }
wasm-bindgen-test = "0.3.33"
base64 = "0.21.2"
base64 = "0.21.2"

[patch.crates-io]
acvm = { git = "https://github.com/noir-lang/acvm.git", rev = "4b4e8a6a25b27e82bddbfe58caa3d421285b4a82" }
acvm-backend-barretenberg = { git = "https://github.com/noir-lang/acvm-backend-barretenberg.git", rev = "c80aee4434a2db502ed8b17ddd0b555f8f3e7c4b" }
3 changes: 2 additions & 1 deletion crates/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ pub fn execute_circuit<B: BlackBoxFunctionSolver + Default>(
initial_witness: WitnessMap,
show_output: bool,
) -> Result<WitnessMap, NargoError> {
let mut acvm = ACVM::new(B::default(), circuit.opcodes, initial_witness);
let backend = B::default();
let mut acvm = ACVM::new(&backend, circuit.opcodes, initial_witness);

loop {
let solver_status = acvm.solve();
Expand Down
6 changes: 3 additions & 3 deletions crates/nargo_cli/src/cli/execute_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub(crate) struct ExecuteCommand {
compile_options: CompileOptions,
}

pub(crate) fn run<B: Backend>(
pub(crate) fn run<B: Backend + acvm::BlackBoxFunctionSolver>(
backend: &B,
args: ExecuteCommand,
config: NargoConfig,
Expand Down Expand Up @@ -70,7 +70,7 @@ pub(crate) fn run<B: Backend>(
Ok(())
}

fn execute_package<B: Backend>(
fn execute_package<B: Backend + acvm::BlackBoxFunctionSolver>(
backend: &B,
package: &Package,
prover_name: &str,
Expand Down Expand Up @@ -161,7 +161,7 @@ fn report_error_with_opcode_location(
}
}

pub(crate) fn execute_program<B: Backend>(
pub(crate) fn execute_program<B: Backend + acvm::BlackBoxFunctionSolver>(
backend: &B,
circuit: Circuit,
abi: &Abi,
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/prove_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) struct ProveCommand {
compile_options: CompileOptions,
}

pub(crate) fn run<B: Backend>(
pub(crate) fn run<B: Backend + acvm::BlackBoxFunctionSolver>(
backend: &B,
args: ProveCommand,
config: NargoConfig,
Expand Down Expand Up @@ -82,7 +82,7 @@ pub(crate) fn run<B: Backend>(
}

#[allow(clippy::too_many_arguments)]
pub(crate) fn prove_package<B: Backend>(
pub(crate) fn prove_package<B: Backend + acvm::BlackBoxFunctionSolver>(
backend: &B,
package: &Package,
prover_name: &str,
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(crate) struct TestCommand {
compile_options: CompileOptions,
}

pub(crate) fn run<B: Backend>(
pub(crate) fn run<B: Backend + acvm::BlackBoxFunctionSolver>(
backend: &B,
args: TestCommand,
config: NargoConfig,
Expand Down Expand Up @@ -73,7 +73,7 @@ pub(crate) fn run<B: Backend>(
Ok(())
}

fn run_tests<B: Backend>(
fn run_tests<B: Backend + acvm::BlackBoxFunctionSolver>(
backend: &B,
package: &Package,
test_name: FunctionNameMatch,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
fn main(mut x: [u32; 5], z: Field) {
let idx = z + 10;

x[z] = 4;

// Dynamic index is greater than length of the array
if z == 20 {
x[0] = x[4];
} else {
// We should not hit out of bounds here as we have a predicate
// that should not be hit
if idx as u32 < 3 {
x[idx] = 10;
}
}
// Error should occur here as an index out of bounds read occurs
assert(x[idx] != 0);

// TODO(#2133): Provide more accurate call stacks for arrays merged in if statements
// if z != 20 {
// x[0] = x[4];
// } else {
// // TODO: Dynamic predicate still gives index out of bounds error
// if idx as u32 < 3 {
// x[idx] = 10;
// }
// x[idx] = 10;
// for i in 0..5 {
// x[idx] = x[i];
// }
// }
// assert(x[idx] != 0);
}
11 changes: 11 additions & 0 deletions crates/nargo_cli/tests/execution_success/array_dynamic/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ fn main(x: [u32; 5], mut z: u32, t: u32, index: [Field;5], index2: [Field;5], of
if 3 < (sublen as u32) {
assert(index[offset + 3] == index2[3]);
}

regression_mem_op_predicate(x, idx + 10);
}

fn dyn_array(mut x: [u32; 5], y: Field, z: Field) {
Expand All @@ -30,3 +32,12 @@ fn dyn_array(mut x: [u32; 5], y: Field, z: Field) {
}
assert(x[4] == 109);
}

fn regression_mem_op_predicate(mut x: [u32; 5], idx: Field) {
// We should not hit out of bounds here as we have a predicate
// that should not be hit
if idx as u32 < 3 {
x[idx] = 10;
}
assert(x[4] == 111);
}
31 changes: 24 additions & 7 deletions crates/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -932,13 +932,13 @@ impl AcirContext {
AcirValue::Array(vars) => {
let mut var_expressions: Vec<Expression> = Vec::new();
for var in vars {
self.brillig_array_input(&mut var_expressions, var)?;
self.brillig_array_input(&mut var_expressions, var, predicate)?;
}
Ok(BrilligInputs::Array(var_expressions))
}
AcirValue::DynamicArray(_) => {
let mut var_expressions = Vec::new();
self.brillig_array_input(&mut var_expressions, i)?;
self.brillig_array_input(&mut var_expressions, i, predicate)?;
Ok(BrilligInputs::Array(var_expressions))
}
})?;
Expand Down Expand Up @@ -977,14 +977,15 @@ impl AcirContext {
&mut self,
var_expressions: &mut Vec<Expression>,
input: AcirValue,
predicate: AcirVar,
) -> Result<(), InternalError> {
match input {
AcirValue::Var(var, _) => {
var_expressions.push(self.var_to_expression(var)?);
}
AcirValue::Array(vars) => {
for var in vars {
self.brillig_array_input(var_expressions, var)?;
self.brillig_array_input(var_expressions, var, predicate)?;
}
}
AcirValue::DynamicArray(AcirDynamicArray { block_id, len }) => {
Expand All @@ -996,13 +997,13 @@ impl AcirContext {
);
let index_var = index.into_var()?;

let value_read_var = self.read_from_memory(block_id, &index_var)?;
let value_read_var = self.read_from_memory(block_id, &index_var, &predicate)?;
let value_read = AcirValue::Var(
value_read_var,
AcirType::NumericType(NumericType::NativeField),
);

self.brillig_array_input(var_expressions, value_read)?;
self.brillig_array_input(var_expressions, value_read, predicate)?;
}
}
}
Expand Down Expand Up @@ -1149,6 +1150,7 @@ impl AcirContext {
&mut self,
block_id: BlockId,
index: &AcirVar,
predicate: &AcirVar,
) -> Result<AcirVar, InternalError> {
// Fetch the witness corresponding to the index
let index_witness = self.var_to_witness(*index)?;
Expand All @@ -1157,9 +1159,16 @@ impl AcirContext {
let value_read_var = self.add_variable();
let value_read_witness = self.var_to_witness(value_read_var)?;

// Fetch the witness corresponding to the predicate
let predicate_witness = self.var_to_witness(*predicate)?;

// Add the memory read operation to the list of opcodes
let op = MemOp::read_at_mem_index(index_witness.into(), value_read_witness);
self.acir_ir.push_opcode(Opcode::MemoryOp { block_id, op });
self.acir_ir.push_opcode(Opcode::MemoryOp {
block_id,
op,
predicate: Some(predicate_witness.into()),
});

Ok(value_read_var)
}
Expand All @@ -1170,6 +1179,7 @@ impl AcirContext {
block_id: BlockId,
index: &AcirVar,
value: &AcirVar,
predicate: &AcirVar,
) -> Result<(), InternalError> {
// Fetch the witness corresponding to the index
//
Expand All @@ -1178,9 +1188,16 @@ impl AcirContext {
// Fetch the witness corresponding to the value to be written
let value_write_witness = self.var_to_witness(*value)?;

// Fetch the witness corresponding to the predicate
let predicate_witness = self.var_to_witness(*predicate)?;

// Add the memory write operation to the list of opcodes
let op = MemOp::write_to_mem_index(index_witness.into(), value_write_witness.into());
self.acir_ir.push_opcode(Opcode::MemoryOp { block_id, op });
self.acir_ir.push_opcode(Opcode::MemoryOp {
block_id,
op,
predicate: Some(predicate_witness.into()),
});

Ok(())
}
Expand Down
25 changes: 21 additions & 4 deletions crates/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,11 @@ impl Context {
);
let index_var = index.into_var()?;

self.acir_context.read_from_memory(block_id, &index_var)
self.acir_context.read_from_memory(
block_id,
&index_var,
&self.current_side_effects_enabled_var,
)
};

for (lhs, rhs) in
Expand Down Expand Up @@ -622,7 +626,11 @@ impl Context {
}

let index_var = self.convert_value(index, dfg).into_var()?;
let read = self.acir_context.read_from_memory(block_id, &index_var)?;
let read = self.acir_context.read_from_memory(
block_id,
&index_var,
&self.current_side_effects_enabled_var,
)?;
let typ = match dfg.type_of_value(array) {
Type::Array(typ, _) => {
if typ.len() != 1 {
Expand Down Expand Up @@ -703,7 +711,11 @@ impl Context {
AcirType::NumericType(NumericType::NativeField),
);
let var = index.into_var()?;
let read = self.acir_context.read_from_memory(block_id, &var)?;
let read = self.acir_context.read_from_memory(
block_id,
&var,
&self.current_side_effects_enabled_var,
)?;
Ok(AcirValue::Var(read, AcirType::NumericType(NumericType::NativeField)))
})?;
self.initialize_array(result_block_id, len, Some(&init_values))?;
Expand All @@ -712,7 +724,12 @@ impl Context {
// Write the new value into the new array at the specified index
let index_var = self.convert_value(index, dfg).into_var()?;
let value_var = self.convert_value(store_value, dfg).into_var()?;
self.acir_context.write_to_memory(result_block_id, &index_var, &value_var)?;
self.acir_context.write_to_memory(
result_block_id,
&index_var,
&value_var,
&self.current_side_effects_enabled_var,
)?;

let result_value =
AcirValue::DynamicArray(AcirDynamicArray { block_id: result_block_id, len });
Expand Down
Loading