From 247d71ba7eb82b1913cdd977f2a07979840c2472 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 20 Dec 2023 12:03:48 +0000 Subject: [PATCH 01/16] feat: decompose constraints on boolean multiplications --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 22 ++++++++--- .../noirc_evaluator/src/ssa/ir/instruction.rs | 37 ++++++++++++++++++- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index abddbfb74c7..57d0967a499 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -166,12 +166,22 @@ impl DataFlowGraph { SimplifiedToMultiple(simplification) } SimplifyResult::Remove => InstructionRemoved, - result @ (SimplifyResult::SimplifiedToInstruction(_) | SimplifyResult::None) => { - let instruction = result.instruction().unwrap_or(instruction); - let id = self.make_instruction(instruction, ctrl_typevars); - self.blocks[block].insert_instruction(id); - self.locations.insert(id, call_stack); - InsertInstructionResult::Results(id, self.instruction_results(id)) + result @ (SimplifyResult::SimplifiedToInstruction(_) + | SimplifyResult::SimplifiedToInstructionMultiple(_) + | SimplifyResult::None) => { + let mut instructions = + result.instructions().unwrap_or(vec![instruction]).into_iter().peekable(); + + while let Some(instruction) = instructions.next() { + let id = self.make_instruction(instruction, ctrl_typevars.clone()); + self.blocks[block].insert_instruction(id); + self.locations.insert(id, call_stack.clone()); + + if instructions.peek().is_none() { + return InsertInstructionResult::Results(id, self.instruction_results(id)); + } + } + unreachable!("more than one instruction must be returned") } } } diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 628ad638e64..343c14e724b 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -472,6 +472,35 @@ impl Instruction { msg.clone(), )) } + + Instruction::Binary(Binary { + lhs, + rhs, + operator: BinaryOp::Mul, + }) if dfg.type_of_value(lhs) == Type::bool() => { + // Replace an equality assertion on a boolean multiplication + // + // v2 = mul v0, v1 + // constrain v2 == u1 1 + // + // with a direct assertion that each value is equal to 1 + // + // v2 = mul v0, v1 + // constrain v0 == 1 + // constrain v1 == 1 + // + // This is due to the fact that for `v2` to be 1 then both `v0` and `v1` are 1. + // + // Note that this doesn't remove the value `v2` as it may be used in other instructions, but it + // will likely be removed through dead instruction elimination. + let one = FieldElement::one(); + let one = dfg.make_constant(one, Type::bool()); + + SimplifiedToInstructionMultiple(vec![ + Instruction::Constrain(lhs, one, msg.clone()), + Instruction::Constrain(rhs, one, msg.clone()), + ]) + } Instruction::Not(value) => { // Replace an assertion that a not instruction is truthy // @@ -1109,6 +1138,9 @@ pub(crate) enum SimplifyResult { /// Replace this function with an simpler but equivalent instruction. SimplifiedToInstruction(Instruction), + /// Replace this function with an simpler but equivalent instruction. + SimplifiedToInstructionMultiple(Vec), + /// Remove the instruction, it is unnecessary Remove, @@ -1117,9 +1149,10 @@ pub(crate) enum SimplifyResult { } impl SimplifyResult { - pub(crate) fn instruction(self) -> Option { + pub(crate) fn instructions(self) -> Option> { match self { - SimplifyResult::SimplifiedToInstruction(instruction) => Some(instruction), + SimplifyResult::SimplifiedToInstruction(instruction) => Some(vec![instruction]), + SimplifyResult::SimplifiedToInstructionMultiple(instructions) => Some(instructions), _ => None, } } From d369b958ee8d0259f07da4ae2050e19d19f89e41 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 20 Dec 2023 12:34:35 +0000 Subject: [PATCH 02/16] fix: add missing check for multiplication to equal 1 --- compiler/noirc_evaluator/src/ssa/ir/instruction.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 343c14e724b..ce20ec3d666 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -477,7 +477,9 @@ impl Instruction { lhs, rhs, operator: BinaryOp::Mul, - }) if dfg.type_of_value(lhs) == Type::bool() => { + }) if constant.is_one() + && dfg.type_of_value(lhs) == Type::bool() => + { // Replace an equality assertion on a boolean multiplication // // v2 = mul v0, v1 From a3e4e8e5cf9117d945d2b93851ba232ead859764 Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 21 Dec 2023 01:51:03 +0000 Subject: [PATCH 03/16] chore: avoid needing multiple rounds of constant folding --- .../noirc_evaluator/src/ssa/ir/instruction.rs | 195 +++++++++--------- 1 file changed, 98 insertions(+), 97 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index ce20ec3d666..f1e125a963a 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -434,104 +434,11 @@ impl Instruction { } } Instruction::Constrain(lhs, rhs, msg) => { - if dfg.resolve(*lhs) == dfg.resolve(*rhs) { - // Remove trivial case `assert_eq(x, x)` - SimplifyResult::Remove + let constraints = decompose_constrain(*lhs, *rhs, msg.clone(), dfg); + if constraints.is_empty() { + Remove } else { - match (&dfg[dfg.resolve(*lhs)], &dfg[dfg.resolve(*rhs)]) { - ( - Value::NumericConstant { constant, typ }, - Value::Instruction { instruction, .. }, - ) - | ( - Value::Instruction { instruction, .. }, - Value::NumericConstant { constant, typ }, - ) if *typ == Type::bool() => { - match dfg[*instruction] { - Instruction::Binary(Binary { - lhs, - rhs, - operator: BinaryOp::Eq, - }) if constant.is_one() => { - // Replace an explicit two step equality assertion - // - // v2 = eq v0, u32 v1 - // constrain v2 == u1 1 - // - // with a direct assertion of equality between the two values - // - // v2 = eq v0, u32 v1 - // constrain v0 == v1 - // - // Note that this doesn't remove the value `v2` as it may be used in other instructions, but it - // will likely be removed through dead instruction elimination. - - SimplifiedToInstruction(Instruction::Constrain( - lhs, - rhs, - msg.clone(), - )) - } - - Instruction::Binary(Binary { - lhs, - rhs, - operator: BinaryOp::Mul, - }) if constant.is_one() - && dfg.type_of_value(lhs) == Type::bool() => - { - // Replace an equality assertion on a boolean multiplication - // - // v2 = mul v0, v1 - // constrain v2 == u1 1 - // - // with a direct assertion that each value is equal to 1 - // - // v2 = mul v0, v1 - // constrain v0 == 1 - // constrain v1 == 1 - // - // This is due to the fact that for `v2` to be 1 then both `v0` and `v1` are 1. - // - // Note that this doesn't remove the value `v2` as it may be used in other instructions, but it - // will likely be removed through dead instruction elimination. - let one = FieldElement::one(); - let one = dfg.make_constant(one, Type::bool()); - - SimplifiedToInstructionMultiple(vec![ - Instruction::Constrain(lhs, one, msg.clone()), - Instruction::Constrain(rhs, one, msg.clone()), - ]) - } - Instruction::Not(value) => { - // Replace an assertion that a not instruction is truthy - // - // v1 = not v0 - // constrain v1 == u1 1 - // - // with an assertion that the not instruction input is falsy - // - // v1 = not v0 - // constrain v0 == u1 0 - // - // Note that this doesn't remove the value `v1` as it may be used in other instructions, but it - // will likely be removed through dead instruction elimination. - let reversed_constant = FieldElement::from(!constant.is_one()); - let reversed_constant = - dfg.make_constant(reversed_constant, Type::bool()); - SimplifiedToInstruction(Instruction::Constrain( - value, - reversed_constant, - msg.clone(), - )) - } - - _ => None, - } - } - - _ => None, - } + SimplifiedToInstructionMultiple(constraints) } } Instruction::ArrayGet { array, index } => { @@ -652,6 +559,100 @@ fn simplify_cast(value: ValueId, dst_typ: &Type, dfg: &mut DataFlowGraph) -> Sim } } +/// Try to decompose this constrain instruction. This constraint will be broken down such that it instead constrains +/// all the values which are used to compute the values which were being constrained. +fn decompose_constrain( + lhs: ValueId, + rhs: ValueId, + msg: Option, + dfg: &mut DataFlowGraph, +) -> Vec { + let lhs = dfg.resolve(lhs); + let rhs = dfg.resolve(rhs); + + if lhs == rhs { + // Remove trivial case `assert_eq(x, x)` + Vec::new() + } else { + match (&dfg[lhs], &dfg[rhs]) { + (Value::NumericConstant { constant, typ }, Value::Instruction { instruction, .. }) + | (Value::Instruction { instruction, .. }, Value::NumericConstant { constant, typ }) + if *typ == Type::bool() => + { + match dfg[*instruction] { + Instruction::Binary(Binary { lhs, rhs, operator: BinaryOp::Eq }) + if constant.is_one() => + { + // Replace an explicit two step equality assertion + // + // v2 = eq v0, u32 v1 + // constrain v2 == u1 1 + // + // with a direct assertion of equality between the two values + // + // v2 = eq v0, u32 v1 + // constrain v0 == v1 + // + // Note that this doesn't remove the value `v2` as it may be used in other instructions, but it + // will likely be removed through dead instruction elimination. + + vec![Instruction::Constrain(lhs, rhs, msg)] + } + + Instruction::Binary(Binary { lhs, rhs, operator: BinaryOp::Mul }) + if constant.is_one() && dfg.type_of_value(lhs) == Type::bool() => + { + // Replace an equality assertion on a boolean multiplication + // + // v2 = mul v0, v1 + // constrain v2 == u1 1 + // + // with a direct assertion that each value is equal to 1 + // + // v2 = mul v0, v1 + // constrain v0 == 1 + // constrain v1 == 1 + // + // This is due to the fact that for `v2` to be 1 then both `v0` and `v1` are 1. + // + // Note that this doesn't remove the value `v2` as it may be used in other instructions, but it + // will likely be removed through dead instruction elimination. + let one = FieldElement::one(); + let one = dfg.make_constant(one, Type::bool()); + + [ + decompose_constrain(lhs, one, msg.clone(), dfg), + decompose_constrain(rhs, one, msg, dfg), + ] + .concat() + } + Instruction::Not(value) => { + // Replace an assertion that a not instruction is truthy + // + // v1 = not v0 + // constrain v1 == u1 1 + // + // with an assertion that the not instruction input is falsy + // + // v1 = not v0 + // constrain v0 == u1 0 + // + // Note that this doesn't remove the value `v1` as it may be used in other instructions, but it + // will likely be removed through dead instruction elimination. + let reversed_constant = FieldElement::from(!constant.is_one()); + let reversed_constant = dfg.make_constant(reversed_constant, Type::bool()); + decompose_constrain(value, reversed_constant, msg, dfg) + } + + _ => vec![Instruction::Constrain(lhs, rhs, msg)], + } + } + + _ => vec![Instruction::Constrain(lhs, rhs, msg)], + } + } +} + /// The possible return values for Instruction::return_types pub(crate) enum InstructionResultType { /// The result type of this instruction matches that of this operand From bd258a8e04b8368581fe6d30df114637a8901a3d Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 21 Dec 2023 02:16:17 +0000 Subject: [PATCH 04/16] chore: handle constraining `(x | y) == 0` --- .../noirc_evaluator/src/ssa/ir/instruction.rs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index f1e125a963a..aff0c36e7ea 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -626,6 +626,35 @@ fn decompose_constrain( ] .concat() } + + Instruction::Binary(Binary { lhs, rhs, operator: BinaryOp::Or }) + if constant.is_zero() => + { + // Replace an equality assertion on an OR + // + // v2 = or v0, v1 + // constrain v2 == u1 0 + // + // with a direct assertion that each value is equal to 0 + // + // v2 = or v0, v1 + // constrain v0 == 0 + // constrain v1 == 0 + // + // This is due to the fact that for `v2` to be 0 then both `v0` and `v1` are 0. + // + // Note that this doesn't remove the value `v2` as it may be used in other instructions, but it + // will likely be removed through dead instruction elimination. + let zero = FieldElement::zero(); + let zero = dfg.make_constant(zero, dfg.type_of_value(lhs)); + + [ + decompose_constrain(lhs, zero, msg.clone(), dfg), + decompose_constrain(rhs, zero, msg, dfg), + ] + .concat() + } + Instruction::Not(value) => { // Replace an assertion that a not instruction is truthy // From 6e656e37a13ca1aad90e0385a7ee6e1b6b21af73 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 2 Jan 2024 15:34:35 +0000 Subject: [PATCH 05/16] chore: add test to track struct equality opcode counts --- .../struct_equality/Nargo.toml | 6 ++++++ .../struct_equality/Prover.toml | 4 ++++ .../struct_equality/src/main.nr | 21 +++++++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 test_programs/execution_success/struct_equality/Nargo.toml create mode 100644 test_programs/execution_success/struct_equality/Prover.toml create mode 100644 test_programs/execution_success/struct_equality/src/main.nr diff --git a/test_programs/execution_success/struct_equality/Nargo.toml b/test_programs/execution_success/struct_equality/Nargo.toml new file mode 100644 index 00000000000..be409842899 --- /dev/null +++ b/test_programs/execution_success/struct_equality/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "struct_equality" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/struct_equality/Prover.toml b/test_programs/execution_success/struct_equality/Prover.toml new file mode 100644 index 00000000000..386062e3994 --- /dev/null +++ b/test_programs/execution_success/struct_equality/Prover.toml @@ -0,0 +1,4 @@ +[x] +bar = 4 +baz = [1, 2, 3] +foo = true diff --git a/test_programs/execution_success/struct_equality/src/main.nr b/test_programs/execution_success/struct_equality/src/main.nr new file mode 100644 index 00000000000..61c8eb4f6d8 --- /dev/null +++ b/test_programs/execution_success/struct_equality/src/main.nr @@ -0,0 +1,21 @@ +// This test demonstrates how SSA optimizations breaks down a struct equality assertion +// to assert directly on the struct's fields rather than using a predicate. + +use dep::std::ops::Eq; + +struct MyStruct { + foo: Field, + bar: bool, + baz: [u8; 3] +} + +impl Eq for MyStruct { + fn eq(self, other: MyStruct) -> bool { + (self.foo == other.foo) & (self.bar == other.bar) & (self.baz == other.baz) + } +} + +fn main(x: MyStruct) { + let y = MyStruct { foo: 42, bar: true, baz: [1, 2, 3] }; + assert(x.eq(y)); +} From c7f97ed6a3a73519cd2d7b33f5ff16a656544b5c Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 2 Jan 2024 15:41:36 +0000 Subject: [PATCH 06/16] chore: got my foos and bars mixed up --- test_programs/execution_success/struct_equality/Prover.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_programs/execution_success/struct_equality/Prover.toml b/test_programs/execution_success/struct_equality/Prover.toml index 386062e3994..31fbf55dd3b 100644 --- a/test_programs/execution_success/struct_equality/Prover.toml +++ b/test_programs/execution_success/struct_equality/Prover.toml @@ -1,4 +1,4 @@ [x] -bar = 4 +bar = true baz = [1, 2, 3] -foo = true +foo = 4 From 24565adf87b5fb85808a1b803d3cebcb960a5b73 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 2 Jan 2024 15:54:15 +0000 Subject: [PATCH 07/16] chore: fix foo value --- test_programs/execution_success/struct_equality/Prover.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_programs/execution_success/struct_equality/Prover.toml b/test_programs/execution_success/struct_equality/Prover.toml index 31fbf55dd3b..08f02c092ac 100644 --- a/test_programs/execution_success/struct_equality/Prover.toml +++ b/test_programs/execution_success/struct_equality/Prover.toml @@ -1,4 +1,4 @@ [x] bar = true baz = [1, 2, 3] -foo = 4 +foo = 42 From 178f2eb3af75a854ed7fb6235c8dcc3ae3c39c41 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 2 Jan 2024 17:39:41 +0000 Subject: [PATCH 08/16] chore: add constraints on usage of `SimplifyResult::SimplifiedToInstructionMultiple` --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 57d0967a499..0b10009926d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -172,6 +172,16 @@ impl DataFlowGraph { let mut instructions = result.instructions().unwrap_or(vec![instruction]).into_iter().peekable(); + if instructions.len() > 1 { + // There's currently no way to pass results from one instruction in `instructions` on to the next. + // We then restrict this to only support multiple instructions if they're all `Instruction::Constrain` + // as this instruction type does not have any results. + assert!( + instructions.all(|instruction| matches!(instruction, Instruction::Constrain(..))), + "`SimplifyResult::SimplifiedToInstructionMultiple` only supports `Constrain` instructions" + ); + } + while let Some(instruction) = instructions.next() { let id = self.make_instruction(instruction, ctrl_typevars.clone()); self.blocks[block].insert_instruction(id); @@ -181,7 +191,7 @@ impl DataFlowGraph { return InsertInstructionResult::Results(id, self.instruction_results(id)); } } - unreachable!("more than one instruction must be returned") + unreachable!("At least one instruction must be returned. To not insert an instruction, use `SimplifyResult::Remove`") } } } From b28b8782056973131d7b6246b35ce7f69c1ff239 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 2 Jan 2024 22:25:01 +0000 Subject: [PATCH 09/16] fix: prevent instructions iterator being consumed when checking length --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 0b10009926d..b319554f37e 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -169,19 +169,20 @@ impl DataFlowGraph { result @ (SimplifyResult::SimplifiedToInstruction(_) | SimplifyResult::SimplifiedToInstructionMultiple(_) | SimplifyResult::None) => { - let mut instructions = - result.instructions().unwrap_or(vec![instruction]).into_iter().peekable(); + let mut instructions = result.instructions().unwrap_or(vec![instruction]); if instructions.len() > 1 { // There's currently no way to pass results from one instruction in `instructions` on to the next. // We then restrict this to only support multiple instructions if they're all `Instruction::Constrain` // as this instruction type does not have any results. assert!( - instructions.all(|instruction| matches!(instruction, Instruction::Constrain(..))), + instructions.iter().all(|instruction| matches!(instruction, Instruction::Constrain(..))), "`SimplifyResult::SimplifiedToInstructionMultiple` only supports `Constrain` instructions" ); } + let mut instructions = instructions.into_iter().peekable(); + while let Some(instruction) = instructions.next() { let id = self.make_instruction(instruction, ctrl_typevars.clone()); self.blocks[block].insert_instruction(id); From 85253aff61b93260159b8df9680c985c64d6de81 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 2 Jan 2024 22:25:15 +0000 Subject: [PATCH 10/16] chore: clippy --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index b319554f37e..4d7ff74f5b2 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -169,7 +169,7 @@ impl DataFlowGraph { result @ (SimplifyResult::SimplifiedToInstruction(_) | SimplifyResult::SimplifiedToInstructionMultiple(_) | SimplifyResult::None) => { - let mut instructions = result.instructions().unwrap_or(vec![instruction]); + let instructions = result.instructions().unwrap_or(vec![instruction]); if instructions.len() > 1 { // There's currently no way to pass results from one instruction in `instructions` on to the next. From b428b81adfa491f75f33ed99e340074f24b00f0c Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:53:06 +0000 Subject: [PATCH 11/16] Update compiler/noirc_evaluator/src/ssa/ir/instruction.rs --- compiler/noirc_evaluator/src/ssa/ir/instruction.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index aff0c36e7ea..9fe2084456a 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -1170,7 +1170,8 @@ pub(crate) enum SimplifyResult { /// Replace this function with an simpler but equivalent instruction. SimplifiedToInstruction(Instruction), - /// Replace this function with an simpler but equivalent instruction. + /// Replace this function with a set of simpler but equivalent instructions. + /// This is currently only to be used for [`Instruction::Constrain`]. SimplifiedToInstructionMultiple(Vec), /// Remove the instruction, it is unnecessary From e3396578fd584eadd01b8204eb827d898d685a3e Mon Sep 17 00:00:00 2001 From: Tom French Date: Thu, 4 Jan 2024 14:15:40 +0000 Subject: [PATCH 12/16] chore: remove broken import --- test_programs/execution_success/struct_equality/src/main.nr | 2 -- 1 file changed, 2 deletions(-) diff --git a/test_programs/execution_success/struct_equality/src/main.nr b/test_programs/execution_success/struct_equality/src/main.nr index 61c8eb4f6d8..cb80ec95311 100644 --- a/test_programs/execution_success/struct_equality/src/main.nr +++ b/test_programs/execution_success/struct_equality/src/main.nr @@ -1,8 +1,6 @@ // This test demonstrates how SSA optimizations breaks down a struct equality assertion // to assert directly on the struct's fields rather than using a predicate. -use dep::std::ops::Eq; - struct MyStruct { foo: Field, bar: bool, From 09190a7b0810e82e784815b0f937e6d3f2911709 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 8 Jan 2024 22:10:47 +0000 Subject: [PATCH 13/16] Update compiler/noirc_evaluator/src/ssa/ir/dfg.rs Co-authored-by: jfecher --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index c86f3f5746e..9efa07ccc56 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -181,18 +181,17 @@ impl DataFlowGraph { ); } - let mut instructions = instructions.into_iter().peekable(); + let last_id = None; - while let Some(instruction) = instructions.next() { + for instruction in instructions { let id = self.make_instruction(instruction, ctrl_typevars.clone()); self.blocks[block].insert_instruction(id); self.locations.insert(id, call_stack.clone()); - - if instructions.peek().is_none() { - return InsertInstructionResult::Results(id, self.instruction_results(id)); - } + last_id = Some(id); } - unreachable!("At least one instruction must be returned. To not insert an instruction, use `SimplifyResult::Remove`") + + let id = last_id.expect("There should be at least 1 simplified instruction"); + InsertInstructionResult::Results(id, self.instruction_results(id)); } } } From d1db6d45a69ae7990f28f5f7dddcd90dc2e0c750 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 8 Jan 2024 22:22:15 +0000 Subject: [PATCH 14/16] chore: fix compilation --- compiler/noirc_evaluator/src/ssa/ir/dfg.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs index 9efa07ccc56..98c0253c6ef 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg.rs @@ -181,7 +181,7 @@ impl DataFlowGraph { ); } - let last_id = None; + let mut last_id = None; for instruction in instructions { let id = self.make_instruction(instruction, ctrl_typevars.clone()); @@ -191,7 +191,7 @@ impl DataFlowGraph { } let id = last_id.expect("There should be at least 1 simplified instruction"); - InsertInstructionResult::Results(id, self.instruction_results(id)); + InsertInstructionResult::Results(id, self.instruction_results(id)) } } } From 519b97bd1a72b59ff2dc98363e4879967b762784 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 8 Jan 2024 22:42:52 +0000 Subject: [PATCH 15/16] chore: add test for constraint decomposition in rust --- .../src/ssa/opt/constant_folding.rs | 65 ++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 57c93c17fc4..2b03109c0b2 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -184,7 +184,7 @@ mod test { function_builder::FunctionBuilder, ir::{ function::RuntimeType, - instruction::{BinaryOp, Instruction, TerminatorInstruction}, + instruction::{Binary, BinaryOp, Instruction, TerminatorInstruction}, map::Id, types::Type, value::Value, @@ -334,4 +334,67 @@ mod test { assert_eq!(instruction, &Instruction::Cast(v0, Type::unsigned(32))); } + + #[test] + fn constraint_decomposition() { + // fn main f0 { + // b0(v0: u1, v1: u1, v2: u1): + // v3 = mul v0 v1 + // v4 = not v2 + // v5 = mul v3 v4 + // constrain v4 u1 1 + // } + // + // When constructing this IR, we should automatically decompose the constraint to be in terms of `v0`, `v1` and `v2`. + // + // The mul instructions are retained and will be removed in the dead instruction elimination pass. + let main_id = Id::test_new(0); + + // Compiling main + let mut builder = FunctionBuilder::new("main".into(), main_id, RuntimeType::Acir); + let v0 = builder.add_parameter(Type::bool()); + let v1 = builder.add_parameter(Type::bool()); + let v2 = builder.add_parameter(Type::bool()); + + let v3 = builder.insert_binary(v0, BinaryOp::Mul, v1); + let v4 = builder.insert_not(v2); + let v5 = builder.insert_binary(v3, BinaryOp::Mul, v4); + + // This constraint is automatically decomposed when it is inserted. + let v_true = builder.numeric_constant(true, Type::bool()); + builder.insert_constrain(v5, v_true, None); + + let v_false = builder.numeric_constant(false, Type::bool()); + + // Expected output: + // + // fn main f0 { + // b0(v0: u1, v1: u1, v2: u1): + // v3 = mul v0 v1 + // v4 = not v2 + // v5 = mul v3 v4 + // constrain v0 u1 1 + // constrain v1 u1 1 + // constrain v2 u1 0 + // } + + let ssa = builder.finish(); + let main = ssa.main(); + let instructions = main.dfg[main.entry_block()].instructions(); + + assert_eq!(instructions.len(), 6); + + assert_eq!( + main.dfg[instructions[0]], + Instruction::Binary(Binary { lhs: v0, operator: BinaryOp::Mul, rhs: v1 }) + ); + assert_eq!(main.dfg[instructions[1]], Instruction::Not(v2)); + assert_eq!( + main.dfg[instructions[2]], + Instruction::Binary(Binary { lhs: v3, operator: BinaryOp::Mul, rhs: v4 }) + ); + assert_eq!(main.dfg[instructions[3]], Instruction::Constrain(v0, v_true, None)); + assert_eq!(main.dfg[instructions[4]], Instruction::Constrain(v1, v_true, None)); + assert_eq!(main.dfg[instructions[5]], Instruction::Constrain(v2, v_false, None)); + } } From d77aecb5bab23199a7fd65aaa85e2476aed38f81 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 8 Jan 2024 22:43:30 +0000 Subject: [PATCH 16/16] chore: remove struct equality test --- .../struct_equality/Nargo.toml | 6 ------ .../struct_equality/Prover.toml | 4 ---- .../struct_equality/src/main.nr | 19 ------------------- 3 files changed, 29 deletions(-) delete mode 100644 test_programs/execution_success/struct_equality/Nargo.toml delete mode 100644 test_programs/execution_success/struct_equality/Prover.toml delete mode 100644 test_programs/execution_success/struct_equality/src/main.nr diff --git a/test_programs/execution_success/struct_equality/Nargo.toml b/test_programs/execution_success/struct_equality/Nargo.toml deleted file mode 100644 index be409842899..00000000000 --- a/test_programs/execution_success/struct_equality/Nargo.toml +++ /dev/null @@ -1,6 +0,0 @@ -[package] -name = "struct_equality" -type = "bin" -authors = [""] - -[dependencies] diff --git a/test_programs/execution_success/struct_equality/Prover.toml b/test_programs/execution_success/struct_equality/Prover.toml deleted file mode 100644 index 08f02c092ac..00000000000 --- a/test_programs/execution_success/struct_equality/Prover.toml +++ /dev/null @@ -1,4 +0,0 @@ -[x] -bar = true -baz = [1, 2, 3] -foo = 42 diff --git a/test_programs/execution_success/struct_equality/src/main.nr b/test_programs/execution_success/struct_equality/src/main.nr deleted file mode 100644 index cb80ec95311..00000000000 --- a/test_programs/execution_success/struct_equality/src/main.nr +++ /dev/null @@ -1,19 +0,0 @@ -// This test demonstrates how SSA optimizations breaks down a struct equality assertion -// to assert directly on the struct's fields rather than using a predicate. - -struct MyStruct { - foo: Field, - bar: bool, - baz: [u8; 3] -} - -impl Eq for MyStruct { - fn eq(self, other: MyStruct) -> bool { - (self.foo == other.foo) & (self.bar == other.bar) & (self.baz == other.baz) - } -} - -fn main(x: MyStruct) { - let y = MyStruct { foo: 42, bar: true, baz: [1, 2, 3] }; - assert(x.eq(y)); -}