From 0181813203a9e3e46c6d8c3169ad5d25971d4282 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Tue, 16 May 2023 11:01:39 +0100 Subject: [PATCH 1/7] chore(noir): Release 0.6.0 (#1279) * chore(noir): Release 0.6.0 * chore: Update lockfile --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++++ Cargo.lock | 22 +++++++++++----------- Cargo.toml | 2 +- flake.nix | 2 +- 4 files changed, 46 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c554330a470..1a8724fe497 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,38 @@ # Changelog +## [0.6.0](https://github.com/noir-lang/noir/compare/v0.5.1...v0.6.0) (2023-05-16) + + +### ⚠ BREAKING CHANGES + +* Update to acvm 0.11.0 ([#1322](https://github.com/noir-lang/noir/issues/1322)) +* **parser:** deprecate `constrain` keyword for `assert` ([#1286](https://github.com/noir-lang/noir/issues/1286)) + +### Features + +* Enable `to_radix` for any field element ([#1343](https://github.com/noir-lang/noir/issues/1343)) ([c3bdec2](https://github.com/noir-lang/noir/commit/c3bdec294234e92a73f39720ec7202fbb17ddc79)) +* Enable dynamic arrays ([#1271](https://github.com/noir-lang/noir/issues/1271)) ([9f43450](https://github.com/noir-lang/noir/commit/9f434507fa431a9dbf4130374b866a5de6176d76)) +* Issue an error when attempting to use a `return` expression ([#1330](https://github.com/noir-lang/noir/issues/1330)) ([a6de557](https://github.com/noir-lang/noir/commit/a6de557e83eb6318d091e40553bb3e2b3823fdc5)) +* **nargo:** Remove usage of `CompiledProgram` in CLI code and use separate ABI/bytecode ([#1269](https://github.com/noir-lang/noir/issues/1269)) ([f144391](https://github.com/noir-lang/noir/commit/f144391b4295b127f3f422e862a087a90dac1dbf)) +* **ssa refactor:** experimental-ssa compiler flag ([#1289](https://github.com/noir-lang/noir/issues/1289)) ([afa6749](https://github.com/noir-lang/noir/commit/afa67494c564b68b667535f2d8ef234fbc4bec12)) +* **ssa refactor:** Implement dominator tree ([#1278](https://github.com/noir-lang/noir/issues/1278)) ([144ebf5](https://github.com/noir-lang/noir/commit/144ebf51522fb19847be28de5595247051fcd92e)) +* **ssa:** add block opcode ([#1291](https://github.com/noir-lang/noir/issues/1291)) ([951ad71](https://github.com/noir-lang/noir/commit/951ad71e0f8bc9a6e95ae21197854396ed7f6e78)) +* **stdlib:** add keccak256 foreign function ([#1249](https://github.com/noir-lang/noir/issues/1249)) ([260d87d](https://github.com/noir-lang/noir/commit/260d87d1ef86069a1fcf0f9b4969589273e381d1)) + + +### Bug Fixes + +* Fix issue with parsing nested generics ([#1319](https://github.com/noir-lang/noir/issues/1319)) ([36f5b8e](https://github.com/noir-lang/noir/commit/36f5b8e88fe8048ece1a54755789d56c8803b3ab)) +* Fix parser error preventing assignments to tuple fields ([#1318](https://github.com/noir-lang/noir/issues/1318)) ([460568e](https://github.com/noir-lang/noir/commit/460568e50a810f90db6559195492547095ab8c32)) +* Fix struct or tuple field assignment failing with generics ([#1317](https://github.com/noir-lang/noir/issues/1317)) ([d872890](https://github.com/noir-lang/noir/commit/d872890e408ada056e9aab84a7774dcaa2049324)), closes [#1315](https://github.com/noir-lang/noir/issues/1315) +* **stdlib:** support use of `to_bits` and `to_radix` for values >128 bits ([#1312](https://github.com/noir-lang/noir/issues/1312)) ([12f3e7e](https://github.com/noir-lang/noir/commit/12f3e7e5917fdcb6b8648032772a7541eaef4751)) + + +### Miscellaneous Chores + +* **parser:** deprecate `constrain` keyword for `assert` ([#1286](https://github.com/noir-lang/noir/issues/1286)) ([9740f54](https://github.com/noir-lang/noir/commit/9740f54c28f30ea9367897fa986d8aea1aba79f2)) +* Update to acvm 0.11.0 ([#1322](https://github.com/noir-lang/noir/issues/1322)) ([da47368](https://github.com/noir-lang/noir/commit/da473685524fc6e5e17f9c3eb95116378ac41fb8)) + ## [0.5.1](https://github.com/noir-lang/noir/compare/v0.5.0...v0.5.1) (2023-05-01) diff --git a/Cargo.lock b/Cargo.lock index c7a30e3adc0..36e2539b098 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -140,7 +140,7 @@ checksum = "23ea9e81bd02e310c216d080f6223c179012256e5151c41db88d12c88a1684d2" [[package]] name = "arena" -version = "0.5.1" +version = "0.6.0" dependencies = [ "generational-arena", ] @@ -1171,7 +1171,7 @@ dependencies = [ [[package]] name = "fm" -version = "0.5.1" +version = "0.6.0" dependencies = [ "cfg-if 1.0.0", "codespan-reporting 0.9.5", @@ -1648,7 +1648,7 @@ dependencies = [ [[package]] name = "iter-extended" -version = "0.5.1" +version = "0.6.0" [[package]] name = "itertools" @@ -1857,7 +1857,7 @@ checksum = "7843ec2de400bcbc6a6328c958dc38e5359da6e93e72e37bc5246bf1ae776389" [[package]] name = "nargo" -version = "0.5.1" +version = "0.6.0" dependencies = [ "acvm", "iter-extended", @@ -1871,7 +1871,7 @@ dependencies = [ [[package]] name = "nargo_cli" -version = "0.5.1" +version = "0.6.0" dependencies = [ "acvm", "acvm-backend-barretenberg", @@ -1902,7 +1902,7 @@ dependencies = [ [[package]] name = "noir_wasm" -version = "0.5.1" +version = "0.6.0" dependencies = [ "acvm", "build-data", @@ -1918,7 +1918,7 @@ dependencies = [ [[package]] name = "noirc_abi" -version = "0.5.1" +version = "0.6.0" dependencies = [ "acvm", "iter-extended", @@ -1932,7 +1932,7 @@ dependencies = [ [[package]] name = "noirc_driver" -version = "0.5.1" +version = "0.6.0" dependencies = [ "acvm", "clap", @@ -1947,7 +1947,7 @@ dependencies = [ [[package]] name = "noirc_errors" -version = "0.5.1" +version = "0.6.0" dependencies = [ "chumsky", "codespan", @@ -1958,7 +1958,7 @@ dependencies = [ [[package]] name = "noirc_evaluator" -version = "0.5.1" +version = "0.6.0" dependencies = [ "acvm", "arena", @@ -1974,7 +1974,7 @@ dependencies = [ [[package]] name = "noirc_frontend" -version = "0.5.1" +version = "0.6.0" dependencies = [ "acvm", "arena", diff --git a/Cargo.toml b/Cargo.toml index 4169aa9b6eb..1b9b9d61f90 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ default-members = ["crates/nargo_cli"] [workspace.package] # x-release-please-start-version -version = "0.5.1" +version = "0.6.0" # x-release-please-end authors = ["The Noir Team "] edition = "2021" diff --git a/flake.nix b/flake.nix index 7109e266e0f..c15a1e51061 100644 --- a/flake.nix +++ b/flake.nix @@ -106,7 +106,7 @@ commonArgs = environment // { pname = "noir"; # x-release-please-start-version - version = "0.5.1"; + version = "0.6.0"; # x-release-please-end # Use our custom stdenv to build and test our Rust project From 63d84a30fcbc117443cd3b404e872cb3c2f26111 Mon Sep 17 00:00:00 2001 From: jfecher Date: Tue, 16 May 2023 07:12:14 -0400 Subject: [PATCH 2/7] chore(ssa refactor): Add basic instruction simplification (#1329) * Add basic instruction simplification * Cargo fmt * Add comments --- .../src/ssa_refactor/ir/dfg.rs | 70 +++++- .../src/ssa_refactor/ir/instruction.rs | 202 +++++++++++++++++- .../src/ssa_refactor/opt/inlining.rs | 19 +- .../src/ssa_refactor/ssa_builder/mod.rs | 19 +- 4 files changed, 285 insertions(+), 25 deletions(-) diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs index 3ab345f06b9..fc15f3e2168 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs @@ -100,15 +100,21 @@ impl DataFlowGraph { id } - /// Replace an instruction id with another. - /// - /// This function should generally be avoided if possible in favor of inserting new - /// instructions since it does not check whether the instruction results of the removed - /// instruction are still in use. Users of this function thus need to ensure the old - /// instruction's results are no longer in use or are otherwise compatible with the - /// new instruction's result count and types. - pub(crate) fn replace_instruction(&mut self, id: Id, instruction: Instruction) { - self.instructions[id] = instruction; + /// Inserts a new instruction at the end of the given block and returns its results + pub(crate) fn insert_instruction( + &mut self, + instruction: Instruction, + block: BasicBlockId, + ctrl_typevars: Option>, + ) -> InsertInstructionResult { + match instruction.simplify(self) { + Some(simplification) => InsertInstructionResult::SimplifiedTo(simplification), + None => { + let id = self.make_instruction(instruction, ctrl_typevars); + self.insert_instruction_in_block(block, id); + InsertInstructionResult::Results(self.instruction_results(id)) + } + } } /// Insert a value into the dfg's storage and return an id to reference it. @@ -300,6 +306,52 @@ impl std::ops::IndexMut for DataFlowGraph { } } +// The result of calling DataFlowGraph::insert_instruction can +// be a list of results or a single ValueId if the instruction was simplified +// to an existing value. +pub(crate) enum InsertInstructionResult<'dfg> { + Results(&'dfg [ValueId]), + SimplifiedTo(ValueId), + InstructionRemoved, +} + +impl<'dfg> InsertInstructionResult<'dfg> { + /// Retrieve the first (and expected to be the only) result. + pub(crate) fn first(&self) -> ValueId { + match self { + InsertInstructionResult::SimplifiedTo(value) => *value, + InsertInstructionResult::Results(results) => results[0], + InsertInstructionResult::InstructionRemoved => { + panic!("Instruction was removed, no results") + } + } + } + + /// Return all the results contained in the internal results array. + /// This is used for instructions returning multiple results that were + /// not simplified - like function calls. + pub(crate) fn results(&self) -> &'dfg [ValueId] { + match self { + InsertInstructionResult::Results(results) => results, + InsertInstructionResult::SimplifiedTo(_) => { + panic!("InsertInstructionResult::results called on a simplified instruction") + } + InsertInstructionResult::InstructionRemoved => { + panic!("InsertInstructionResult::results called on a removed instruction") + } + } + } + + /// Returns the amount of ValueIds contained + pub(crate) fn len(&self) -> usize { + match self { + InsertInstructionResult::SimplifiedTo(_) => 1, + InsertInstructionResult::Results(results) => results.len(), + InsertInstructionResult::InstructionRemoved => 0, + } + } +} + #[cfg(test)] mod tests { use super::DataFlowGraph; diff --git a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs index 812d12b23a3..42968568dee 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ir/instruction.rs @@ -1,7 +1,13 @@ -use acvm::acir::BlackBoxFunc; +use acvm::{acir::BlackBoxFunc, FieldElement}; use iter_extended::vecmap; -use super::{basic_block::BasicBlockId, map::Id, types::Type, value::ValueId}; +use super::{ + basic_block::BasicBlockId, + dfg::DataFlowGraph, + map::Id, + types::Type, + value::{Value, ValueId}, +}; /// Reference to an instruction /// @@ -151,6 +157,48 @@ impl Instruction { } } } + + /// Try to simplify this instruction. If the instruction can be simplified to a known value, + /// that value is returned. Otherwise None is returned. + pub(crate) fn simplify(&self, dfg: &mut DataFlowGraph) -> Option { + match self { + Instruction::Binary(binary) => binary.simplify(dfg), + Instruction::Cast(value, typ) => (*typ == dfg.type_of_value(*value)).then_some(*value), + Instruction::Not(value) => { + match &dfg[*value] { + // Limit optimizing ! on constants to only booleans. If we tried it on fields, + // there is no Not on FieldElement, so we'd need to convert between u128. This + // would be incorrect however since the extra bits on the field would not be flipped. + Value::NumericConstant { constant, typ } if *typ == Type::bool() => { + let value = dfg[*constant].value().is_zero() as u128; + Some(dfg.make_constant(value.into(), Type::bool())) + } + Value::Instruction { instruction, .. } => { + // !!v => v + match &dfg[*instruction] { + Instruction::Not(value) => Some(*value), + _ => None, + } + } + _ => None, + } + } + Instruction::Constrain(value) => { + if let Some(constant) = dfg.get_numeric_constant(*value) { + if constant.is_one() { + // "simplify" to a unit literal that will just be thrown away anyway + return Some(dfg.make_constant(0u128.into(), Type::Unit)); + } + } + None + } + Instruction::Truncate { .. } => None, + Instruction::Call { .. } => None, + Instruction::Allocate { .. } => None, + Instruction::Load { .. } => None, + Instruction::Store { .. } => None, + } + } } /// The possible return values for Instruction::return_types @@ -219,6 +267,156 @@ impl Binary { _ => InstructionResultType::Operand(self.lhs), } } + + /// Try to simplify this binary instruction, returning the new value if possible. + fn simplify(&self, dfg: &mut DataFlowGraph) -> Option { + let lhs = dfg.get_numeric_constant(self.lhs); + let rhs = dfg.get_numeric_constant(self.rhs); + let operand_type = dfg.type_of_value(self.lhs); + + if let (Some(lhs), Some(rhs)) = (lhs, rhs) { + return self.eval_constants(dfg, lhs, rhs, operand_type); + } + + let lhs_is_zero = lhs.map_or(false, |lhs| lhs.is_zero()); + let rhs_is_zero = rhs.map_or(false, |rhs| rhs.is_zero()); + + let lhs_is_one = lhs.map_or(false, |lhs| lhs.is_one()); + let rhs_is_one = rhs.map_or(false, |rhs| rhs.is_one()); + + match self.operator { + BinaryOp::Add => { + if lhs_is_zero { + return Some(self.rhs); + } + if rhs_is_zero { + return Some(self.lhs); + } + } + BinaryOp::Sub => { + if rhs_is_zero { + return Some(self.lhs); + } + } + BinaryOp::Mul => { + if lhs_is_one { + return Some(self.rhs); + } + if rhs_is_one { + return Some(self.lhs); + } + } + BinaryOp::Div => { + if rhs_is_one { + return Some(self.lhs); + } + } + BinaryOp::Mod => { + if rhs_is_one { + return Some(self.lhs); + } + } + BinaryOp::Eq => { + if self.lhs == self.rhs { + return Some(dfg.make_constant(FieldElement::one(), Type::bool())); + } + } + BinaryOp::Lt => { + if self.lhs == self.rhs { + return Some(dfg.make_constant(FieldElement::zero(), Type::bool())); + } + } + BinaryOp::And => { + if lhs_is_zero || rhs_is_zero { + return Some(dfg.make_constant(FieldElement::zero(), operand_type)); + } + } + BinaryOp::Or => { + if lhs_is_zero { + return Some(self.rhs); + } + if rhs_is_zero { + return Some(self.lhs); + } + } + BinaryOp::Xor => (), + BinaryOp::Shl => { + if rhs_is_zero { + return Some(self.lhs); + } + } + BinaryOp::Shr => { + if rhs_is_zero { + return Some(self.lhs); + } + } + } + None + } + + /// Evaluate the two constants with the operation specified by self.operator. + /// Pushes the resulting value to the given DataFlowGraph's constants and returns it. + fn eval_constants( + &self, + dfg: &mut DataFlowGraph, + lhs: FieldElement, + rhs: FieldElement, + operand_type: Type, + ) -> Option> { + let value = match self.operator { + BinaryOp::Add => lhs + rhs, + BinaryOp::Sub => lhs - rhs, + BinaryOp::Mul => lhs * rhs, + BinaryOp::Div => lhs / rhs, + BinaryOp::Eq => (lhs == rhs).into(), + BinaryOp::Lt => (lhs < rhs).into(), + + // The rest of the operators we must try to convert to u128 first + BinaryOp::Mod => self.eval_constant_u128_operations(lhs, rhs)?, + BinaryOp::And => self.eval_constant_u128_operations(lhs, rhs)?, + BinaryOp::Or => self.eval_constant_u128_operations(lhs, rhs)?, + BinaryOp::Xor => self.eval_constant_u128_operations(lhs, rhs)?, + BinaryOp::Shl => self.eval_constant_u128_operations(lhs, rhs)?, + BinaryOp::Shr => self.eval_constant_u128_operations(lhs, rhs)?, + }; + // TODO: Keep original type of constant + Some(dfg.make_constant(value, operand_type)) + } + + /// Try to evaluate the given operands as u128s for operators that are only valid on u128s, + /// like the bitwise operators and modulus. + fn eval_constant_u128_operations( + &self, + lhs: FieldElement, + rhs: FieldElement, + ) -> Option { + let lhs = lhs.try_into_u128()?; + let rhs = rhs.try_into_u128()?; + match self.operator { + BinaryOp::Mod => Some((lhs % rhs).into()), + BinaryOp::And => Some((lhs & rhs).into()), + BinaryOp::Or => Some((lhs | rhs).into()), + BinaryOp::Shr => Some((lhs >> rhs).into()), + // Check for overflow and return None if anything does overflow + BinaryOp::Shl => { + let rhs = rhs.try_into().ok()?; + lhs.checked_shl(rhs).map(Into::into) + } + + // Converting a field xor to a u128 xor would be incorrect since we wouldn't have the + // extra bits of the field. So we don't optimize it here. + BinaryOp::Xor => None, + + op @ (BinaryOp::Add + | BinaryOp::Sub + | BinaryOp::Mul + | BinaryOp::Div + | BinaryOp::Eq + | BinaryOp::Lt) => panic!( + "eval_constant_u128_operations invalid for {op:?} use eval_constants instead" + ), + } + } } /// Binary Operations allowed in the IR. diff --git a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs index 50c97b765bb..c63cac520bf 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/opt/inlining.rs @@ -9,6 +9,7 @@ use iter_extended::vecmap; use crate::ssa_refactor::{ ir::{ basic_block::BasicBlockId, + dfg::InsertInstructionResult, function::{Function, FunctionId}, instruction::{Instruction, InstructionId, TerminatorInstruction}, value::{Value, ValueId}, @@ -343,7 +344,8 @@ impl<'function> PerFunctionContext<'function> { let old_results = self.source_function.dfg.instruction_results(call_id); let arguments = vecmap(arguments, |arg| self.translate_value(*arg)); let new_results = self.context.inline_function(ssa, function, &arguments); - Self::insert_new_instruction_results(&mut self.values, old_results, &new_results); + let new_results = InsertInstructionResult::Results(&new_results); + Self::insert_new_instruction_results(&mut self.values, old_results, new_results); } /// Push the given instruction from the source_function into the current block of the @@ -365,11 +367,20 @@ impl<'function> PerFunctionContext<'function> { fn insert_new_instruction_results( values: &mut HashMap, old_results: &[ValueId], - new_results: &[ValueId], + new_results: InsertInstructionResult, ) { assert_eq!(old_results.len(), new_results.len()); - for (old_result, new_result) in old_results.iter().zip(new_results) { - values.insert(*old_result, *new_result); + + match new_results { + InsertInstructionResult::SimplifiedTo(new_result) => { + values.insert(old_results[0], new_result); + } + InsertInstructionResult::Results(new_results) => { + for (old_result, new_result) in old_results.iter().zip(new_results) { + values.insert(*old_result, *new_result); + } + } + InsertInstructionResult::InstructionRemoved => (), } } diff --git a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs index f621503e59a..60379097523 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/ssa_builder/mod.rs @@ -11,6 +11,7 @@ use crate::ssa_refactor::ir::{ use super::{ ir::{ basic_block::BasicBlock, + dfg::InsertInstructionResult, instruction::{InstructionId, Intrinsic}, }, ssa_gen::Ssa, @@ -108,10 +109,8 @@ impl FunctionBuilder { &mut self, instruction: Instruction, ctrl_typevars: Option>, - ) -> &[ValueId] { - let id = self.current_function.dfg.make_instruction(instruction, ctrl_typevars); - self.current_function.dfg.insert_instruction_in_block(self.current_block, id); - self.current_function.dfg.instruction_results(id) + ) -> InsertInstructionResult { + self.current_function.dfg.insert_instruction(instruction, self.current_block, ctrl_typevars) } /// Switch to inserting instructions in the given block. @@ -130,7 +129,7 @@ impl FunctionBuilder { /// given amount of field elements. Returns the result of the allocate instruction, /// which is always a Reference to the allocated data. pub(crate) fn insert_allocate(&mut self, size_to_allocate: u32) -> ValueId { - self.insert_instruction(Instruction::Allocate { size: size_to_allocate }, None)[0] + self.insert_instruction(Instruction::Allocate { size: size_to_allocate }, None).first() } /// Insert a Load instruction at the end of the current block, loading from the given offset @@ -147,7 +146,7 @@ impl FunctionBuilder { type_to_load: Type, ) -> ValueId { address = self.insert_binary(address, BinaryOp::Add, offset); - self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load]))[0] + self.insert_instruction(Instruction::Load { address }, Some(vec![type_to_load])).first() } /// Insert a Store instruction at the end of the current block, storing the given element @@ -166,19 +165,19 @@ impl FunctionBuilder { rhs: ValueId, ) -> ValueId { let instruction = Instruction::Binary(Binary { lhs, rhs, operator }); - self.insert_instruction(instruction, None)[0] + self.insert_instruction(instruction, None).first() } /// Insert a not instruction at the end of the current block. /// Returns the result of the instruction. pub(crate) fn insert_not(&mut self, rhs: ValueId) -> ValueId { - self.insert_instruction(Instruction::Not(rhs), None)[0] + self.insert_instruction(Instruction::Not(rhs), None).first() } /// Insert a cast instruction at the end of the current block. /// Returns the result of the cast instruction. pub(crate) fn insert_cast(&mut self, value: ValueId, typ: Type) -> ValueId { - self.insert_instruction(Instruction::Cast(value, typ), None)[0] + self.insert_instruction(Instruction::Cast(value, typ), None).first() } /// Insert a constrain instruction at the end of the current block. @@ -194,7 +193,7 @@ impl FunctionBuilder { arguments: Vec, result_types: Vec, ) -> &[ValueId] { - self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)) + self.insert_instruction(Instruction::Call { func, arguments }, Some(result_types)).results() } /// Terminates the current block with the given terminator instruction From a830ce5c3b247a7a60d7d4de4f8470e3227b8a47 Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Tue, 16 May 2023 15:41:57 +0200 Subject: [PATCH 3/7] chore(ssa): enable cse for assert (#1350) enable cse for assert --- crates/noirc_evaluator/src/ssa/node.rs | 30 +++++++++++++++++++ .../noirc_evaluator/src/ssa/optimizations.rs | 16 ++++++++++ 2 files changed, 46 insertions(+) diff --git a/crates/noirc_evaluator/src/ssa/node.rs b/crates/noirc_evaluator/src/ssa/node.rs index bec3c923a6d..4566d974813 100644 --- a/crates/noirc_evaluator/src/ssa/node.rs +++ b/crates/noirc_evaluator/src/ssa/node.rs @@ -506,6 +506,36 @@ impl Instruction { } } } + + pub(crate) fn get_location(&self) -> Option { + match &self.operation { + Operation::Binary(bin) => match bin.operator { + BinaryOp::Udiv(location) + | BinaryOp::Sdiv(location) + | BinaryOp::Urem(location) + | BinaryOp::Srem(location) + | BinaryOp::Div(location) + | BinaryOp::Shr(location) => Some(location), + _ => None, + }, + Operation::Call { location, .. } => Some(*location), + Operation::Load { location, .. } + | Operation::Store { location, .. } + | Operation::Constrain(_, location) => *location, + Operation::Cast(_) + | Operation::Truncate { .. } + | Operation::Not(_) + | Operation::Jne(_, _) + | Operation::Jeq(_, _) + | Operation::Jmp(_) + | Operation::Phi { .. } + | Operation::Return(_) + | Operation::Result { .. } + | Operation::Cond { .. } + | Operation::Intrinsic(_, _) + | Operation::Nop => None, + } + } } //adapted from LLVM IR diff --git a/crates/noirc_evaluator/src/ssa/optimizations.rs b/crates/noirc_evaluator/src/ssa/optimizations.rs index f1cfca9c243..d238ae7b0fe 100644 --- a/crates/noirc_evaluator/src/ssa/optimizations.rs +++ b/crates/noirc_evaluator/src/ssa/optimizations.rs @@ -507,6 +507,22 @@ fn cse_block_with_anchor( new_list.push(*ins_id); } } + Operation::Constrain(condition, location) => { + if let Some(similar) = anchor.find_similar_instruction(&operator) { + assert_ne!(similar, ins.id); + *modified = true; + let similar_ins = ctx + .try_get_mut_instruction(similar) + .expect("Similar instructions are instructions"); + if location.is_some() && similar_ins.get_location().is_none() { + similar_ins.operation = Operation::Constrain(*condition, *location); + } + new_mark = Mark::ReplaceWith(similar); + } else { + new_list.push(*ins_id); + anchor.push_front(&ins.operation, *ins_id); + } + } _ => { //TODO: checks we do not need to propagate res arguments new_list.push(*ins_id); From dffa3c50337ec0f71a62377d985ebdc8eefe490e Mon Sep 17 00:00:00 2001 From: joss-aztec <94053499+joss-aztec@users.noreply.github.com> Date: Tue, 16 May 2023 15:59:12 +0100 Subject: [PATCH 4/7] feat(nargo)!: retire print-acir in favour of flag (#1328) feat!(nargo): retire print-acir in favour of flag --- crates/nargo_cli/src/cli/mod.rs | 8 ++--- crates/nargo_cli/src/cli/print_acir_cmd.rs | 35 ---------------------- crates/nargo_cli/tests/prove_and_verify.rs | 2 +- crates/noirc_driver/src/lib.rs | 34 +++++++++++++++++++-- 4 files changed, 35 insertions(+), 44 deletions(-) delete mode 100644 crates/nargo_cli/src/cli/print_acir_cmd.rs diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index d41dc1a815a..bdb9a926991 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -15,7 +15,6 @@ mod compile_cmd; mod execute_cmd; mod gates_cmd; mod new_cmd; -mod print_acir_cmd; mod prove_cmd; mod test_cmd; mod verify_cmd; @@ -56,7 +55,6 @@ enum NargoCommand { Verify(verify_cmd::VerifyCommand), Test(test_cmd::TestCommand), Gates(gates_cmd::GatesCommand), - PrintAcir(print_acir_cmd::PrintAcirCommand), } pub fn start_cli() -> eyre::Result<()> { @@ -79,18 +77,18 @@ pub fn start_cli() -> eyre::Result<()> { NargoCommand::Test(args) => test_cmd::run(&backend, args, config), NargoCommand::Gates(args) => gates_cmd::run(&backend, args, config), NargoCommand::CodegenVerifier(args) => codegen_verifier_cmd::run(&backend, args, config), - NargoCommand::PrintAcir(args) => print_acir_cmd::run(&backend, args, config), }?; Ok(()) } // helper function which tests noir programs by trying to generate a proof and verify it -pub fn prove_and_verify(proof_name: &str, program_dir: &Path, show_ssa: bool) -> bool { +pub fn prove_and_verify(proof_name: &str, program_dir: &Path) -> bool { let backend = crate::backends::ConcreteBackend::default(); let compile_options = CompileOptions { - show_ssa, + show_ssa: false, + print_acir: false, allow_warnings: false, show_output: false, experimental_ssa: false, diff --git a/crates/nargo_cli/src/cli/print_acir_cmd.rs b/crates/nargo_cli/src/cli/print_acir_cmd.rs deleted file mode 100644 index 420c57c6a08..00000000000 --- a/crates/nargo_cli/src/cli/print_acir_cmd.rs +++ /dev/null @@ -1,35 +0,0 @@ -use acvm::Backend; -use clap::Args; -use noirc_driver::CompileOptions; -use std::path::Path; - -use crate::cli::compile_cmd::compile_circuit; -use crate::errors::CliError; - -use super::NargoConfig; - -/// Prints out the ACIR for a compiled circuit -#[derive(Debug, Clone, Args)] -pub(crate) struct PrintAcirCommand { - #[clap(flatten)] - compile_options: CompileOptions, -} - -pub(crate) fn run( - backend: &B, - args: PrintAcirCommand, - config: NargoConfig, -) -> Result<(), CliError> { - print_acir_with_path(backend, config.program_dir, &args.compile_options) -} - -fn print_acir_with_path>( - backend: &B, - program_dir: P, - compile_options: &CompileOptions, -) -> Result<(), CliError> { - let compiled_program = compile_circuit(backend, program_dir.as_ref(), compile_options)?; - println!("{}", compiled_program.circuit); - - Ok(()) -} diff --git a/crates/nargo_cli/tests/prove_and_verify.rs b/crates/nargo_cli/tests/prove_and_verify.rs index 070db6d8ce8..288073e6c1e 100644 --- a/crates/nargo_cli/tests/prove_and_verify.rs +++ b/crates/nargo_cli/tests/prove_and_verify.rs @@ -80,7 +80,7 @@ mod tests { println!("Running test {test_name}"); let verified = std::panic::catch_unwind(|| { - nargo_cli::cli::prove_and_verify("pp", test_program_dir, false) + nargo_cli::cli::prove_and_verify("pp", test_program_dir) }); let r = match verified { diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index a2fbed21885..c88a1a02b2b 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -36,6 +36,10 @@ pub struct CompileOptions { #[arg(short, long)] pub show_ssa: bool, + /// Display the ACIR for compiled circuit + #[arg(short, long)] + pub print_acir: bool, + /// Issue a warning for each unused variable instead of an error #[arg(short, long)] pub allow_warnings: bool, @@ -51,7 +55,13 @@ pub struct CompileOptions { impl Default for CompileOptions { fn default() -> Self { - Self { show_ssa: false, allow_warnings: false, show_output: true, experimental_ssa: false } + Self { + show_ssa: false, + print_acir: false, + allow_warnings: false, + show_output: true, + experimental_ssa: false, + } } } @@ -188,7 +198,12 @@ impl Driver { return Err(e); } }; - self.compile_no_check(options, main) + let compiled_program = self.compile_no_check(options, main)?; + if options.print_acir { + println!("Compiled ACIR for main:"); + println!("{}", compiled_program.circuit); + } + Ok(compiled_program) } /// Run the frontend to check the crate for errors then compile all contracts if there were none @@ -198,7 +213,20 @@ impl Driver { ) -> Result, ReportedError> { self.check_crate(options)?; let contracts = self.get_all_contracts(); - try_vecmap(contracts, |contract| self.compile_contract(contract, options)) + let compiled_contracts = + try_vecmap(contracts, |contract| self.compile_contract(contract, options))?; + if options.print_acir { + for compiled_contract in &compiled_contracts { + for contract_function in &compiled_contract.functions { + println!( + "Compiled ACIR for {}::{}:", + compiled_contract.name, contract_function.name + ); + println!("{}", contract_function.bytecode); + } + } + } + Ok(compiled_contracts) } /// Compile all of the functions associated with a Noir contract. From 1e958c2aef89328e5354457c2a1e8697486e2978 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 May 2023 06:30:32 +0800 Subject: [PATCH 5/7] feat: pass in closure to `Driver` to signal backend opcode support (#1349) * feat: pass in opcode support function * chore: remove langauge from `NodeInterner` * chore: add comment and replace non-standard `is_opcode_supported`s * chore: prefer usage of `impl` over `dyn` * chore: fix build --- crates/nargo_cli/src/cli/check_cmd.rs | 7 ++++++- crates/nargo_cli/src/cli/compile_cmd.rs | 7 ++++++- crates/nargo_cli/src/cli/mod.rs | 6 +++++- crates/nargo_cli/src/cli/test_cmd.rs | 7 ++++++- crates/nargo_cli/src/resolver.rs | 5 +++-- crates/noirc_driver/src/lib.rs | 16 ++++++++++------ crates/noirc_driver/src/main.rs | 7 ++++++- crates/noirc_frontend/src/hir/mod.rs | 10 +++++++--- crates/noirc_frontend/src/main.rs | 7 ++++++- crates/noirc_frontend/src/node_interner.rs | 12 ++++++------ crates/wasm/src/compile.rs | 6 +++++- 11 files changed, 66 insertions(+), 24 deletions(-) diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs index 9664930466b..c57bb2a1fd9 100644 --- a/crates/nargo_cli/src/cli/check_cmd.rs +++ b/crates/nargo_cli/src/cli/check_cmd.rs @@ -32,7 +32,12 @@ fn check_from_path>( p: P, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = Resolver::resolve_root_manifest(p.as_ref(), backend.np_language())?; + let mut driver = Resolver::resolve_root_manifest( + p.as_ref(), + backend.np_language(), + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(backend.np_language())), + )?; driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; diff --git a/crates/nargo_cli/src/cli/compile_cmd.rs b/crates/nargo_cli/src/cli/compile_cmd.rs index 531560b87db..fbdc9330da8 100644 --- a/crates/nargo_cli/src/cli/compile_cmd.rs +++ b/crates/nargo_cli/src/cli/compile_cmd.rs @@ -63,7 +63,12 @@ fn setup_driver( backend: &B, program_dir: &Path, ) -> Result { - Resolver::resolve_root_manifest(program_dir, backend.np_language()) + Resolver::resolve_root_manifest( + program_dir, + backend.np_language(), + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(backend.np_language())), + ) } pub(crate) fn compile_circuit( diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index bdb9a926991..c61a970905b 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -126,7 +126,11 @@ mod tests { /// /// This is used for tests. fn file_compiles>(root_file: P) -> bool { - let mut driver = Driver::new(&acvm::Language::R1CS); + let mut driver = Driver::new( + &acvm::Language::R1CS, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)), + ); driver.create_local_crate(&root_file, CrateType::Binary); crate::resolver::add_std_lib(&mut driver); driver.file_compiles() diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs index 139d33b6c3d..024eaa20600 100644 --- a/crates/nargo_cli/src/cli/test_cmd.rs +++ b/crates/nargo_cli/src/cli/test_cmd.rs @@ -37,7 +37,12 @@ fn run_tests( test_name: &str, compile_options: &CompileOptions, ) -> Result<(), CliError> { - let mut driver = Resolver::resolve_root_manifest(program_dir, backend.np_language())?; + let mut driver = Resolver::resolve_root_manifest( + program_dir, + backend.np_language(), + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(backend.np_language())), + )?; driver.check_crate(compile_options).map_err(|_| CliError::CompilationError)?; diff --git a/crates/nargo_cli/src/resolver.rs b/crates/nargo_cli/src/resolver.rs index ddd607058ed..2ce26a44b26 100644 --- a/crates/nargo_cli/src/resolver.rs +++ b/crates/nargo_cli/src/resolver.rs @@ -3,7 +3,7 @@ use std::{ path::{Path, PathBuf}, }; -use acvm::Language; +use acvm::{acir::circuit::Opcode, Language}; use nargo::manifest::{Dependency, PackageManifest}; use noirc_driver::Driver; use noirc_frontend::graph::{CrateId, CrateName, CrateType}; @@ -71,8 +71,9 @@ impl<'a> Resolver<'a> { pub(crate) fn resolve_root_manifest( dir_path: &std::path::Path, np_language: Language, + is_opcode_supported: Box bool>, ) -> Result { - let mut driver = Driver::new(&np_language); + let mut driver = Driver::new(&np_language, is_opcode_supported); let (entry_path, crate_type) = super::lib_or_bin(dir_path)?; let manifest_path = super::find_package_manifest(dir_path)?; diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs index c88a1a02b2b..00eb417a36c 100644 --- a/crates/noirc_driver/src/lib.rs +++ b/crates/noirc_driver/src/lib.rs @@ -3,6 +3,7 @@ #![warn(unreachable_pub)] #![warn(clippy::semicolon_if_nothing_returned)] +use acvm::acir::circuit::Opcode; use acvm::Language; use clap::Args; use contract::ContractFunction; @@ -66,9 +67,9 @@ impl Default for CompileOptions { } impl Driver { - pub fn new(np_language: &Language) -> Self { - let mut driver = Driver { context: Context::default(), language: np_language.clone() }; - driver.context.def_interner.set_language(np_language); + pub fn new(language: &Language, is_opcode_supported: Box bool>) -> Self { + let mut driver = Driver { context: Context::default(), language: language.clone() }; + driver.context.def_interner.set_opcode_support(is_opcode_supported); driver } @@ -76,9 +77,10 @@ impl Driver { // with the restricted version which only uses one file pub fn compile_file( root_file: PathBuf, - np_language: acvm::Language, + language: &Language, + is_opcode_supported: Box bool>, ) -> Result { - let mut driver = Driver::new(&np_language); + let mut driver = Driver::new(language, is_opcode_supported); driver.create_local_crate(root_file, CrateType::Binary); driver.compile_main(&CompileOptions::default()) } @@ -284,6 +286,7 @@ impl Driver { let program = monomorphize(main_function, &self.context.def_interner); let np_language = self.language.clone(); + // TODO: use proper `is_opcode_supported` implementation. let is_opcode_supported = acvm::default_is_opcode_supported(np_language.clone()); let circuit_abi = if options.experimental_ssa { @@ -346,6 +349,7 @@ impl Driver { impl Default for Driver { fn default() -> Self { - Self::new(&Language::R1CS) + #[allow(deprecated)] + Self::new(&Language::R1CS, Box::new(acvm::default_is_opcode_supported(Language::R1CS))) } } diff --git a/crates/noirc_driver/src/main.rs b/crates/noirc_driver/src/main.rs index 0d9127d04bd..ca7c5faa808 100644 --- a/crates/noirc_driver/src/main.rs +++ b/crates/noirc_driver/src/main.rs @@ -1,3 +1,4 @@ +use acvm::Language; use noirc_driver::{CompileOptions, Driver}; use noirc_frontend::graph::{CrateType, LOCAL_CRATE}; fn main() { @@ -5,7 +6,11 @@ fn main() { const EXTERNAL_DIR2: &str = "dep_a/lib.nr"; const ROOT_DIR_MAIN: &str = "example_real_project/main.nr"; - let mut driver = Driver::new(&acvm::Language::R1CS); + let mut driver = Driver::new( + &Language::R1CS, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(Language::R1CS)), + ); // Add local crate to dep graph driver.create_local_crate(ROOT_DIR_MAIN, CrateType::Binary); diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs index 6d469a4229d..2ebabf86867 100644 --- a/crates/noirc_frontend/src/hir/mod.rs +++ b/crates/noirc_frontend/src/hir/mod.rs @@ -6,7 +6,7 @@ pub mod type_check; use crate::graph::{CrateGraph, CrateId}; use crate::node_interner::NodeInterner; -use acvm::Language; +use acvm::acir::circuit::Opcode; use def_map::CrateDefMap; use fm::FileManager; use std::collections::HashMap; @@ -29,7 +29,11 @@ pub struct Context { pub type StorageSlot = u32; impl Context { - pub fn new(file_manager: FileManager, crate_graph: CrateGraph, language: Language) -> Context { + pub fn new( + file_manager: FileManager, + crate_graph: CrateGraph, + is_opcode_supported: Box bool>, + ) -> Context { let mut ctx = Context { def_interner: NodeInterner::default(), def_maps: HashMap::new(), @@ -37,7 +41,7 @@ impl Context { file_manager, storage_slots: HashMap::new(), }; - ctx.def_interner.set_language(&language); + ctx.def_interner.set_opcode_support(is_opcode_supported); ctx } diff --git a/crates/noirc_frontend/src/main.rs b/crates/noirc_frontend/src/main.rs index b1a5c0f950e..023a1440e52 100644 --- a/crates/noirc_frontend/src/main.rs +++ b/crates/noirc_frontend/src/main.rs @@ -27,7 +27,12 @@ fn main() { let crate_id = crate_graph.add_crate_root(CrateType::Library, root_file_id); // initiate context with file manager and crate graph - let mut context = Context::new(fm, crate_graph, acvm::Language::R1CS); + let mut context = Context::new( + fm, + crate_graph, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)), + ); // Now create the CrateDefMap // This is preamble for analysis diff --git a/crates/noirc_frontend/src/node_interner.rs b/crates/noirc_frontend/src/node_interner.rs index d8ea11ae89c..f778c3aa552 100644 --- a/crates/noirc_frontend/src/node_interner.rs +++ b/crates/noirc_frontend/src/node_interner.rs @@ -70,7 +70,7 @@ pub struct NodeInterner { next_type_variable_id: usize, //used for fallback mechanism - language: Language, + is_opcode_supported: Box bool>, delayed_type_checks: Vec, @@ -258,7 +258,8 @@ impl Default for NodeInterner { field_indices: HashMap::new(), next_type_variable_id: 0, globals: HashMap::new(), - language: Language::R1CS, + #[allow(deprecated)] + is_opcode_supported: Box::new(acvm::default_is_opcode_supported(Language::R1CS)), delayed_type_checks: vec![], struct_methods: HashMap::new(), primitive_methods: HashMap::new(), @@ -579,18 +580,17 @@ impl NodeInterner { self.function_definition_ids[&function] } - pub fn set_language(&mut self, language: &Language) { - self.language = language.clone(); + pub fn set_opcode_support(&mut self, is_opcode_supported: Box bool>) { + self.is_opcode_supported = is_opcode_supported; } #[allow(deprecated)] pub fn foreign(&self, opcode: &str) -> bool { - let is_supported = acvm::default_is_opcode_supported(self.language.clone()); let black_box_func = match acvm::acir::BlackBoxFunc::lookup(opcode) { Some(black_box_func) => black_box_func, None => return false, }; - is_supported(&Opcode::BlackBoxFuncCall(BlackBoxFuncCall { + (self.is_opcode_supported)(&Opcode::BlackBoxFuncCall(BlackBoxFuncCall { name: black_box_func, inputs: Vec::new(), outputs: Vec::new(), diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs index ecf2b789365..e515372cf3a 100644 --- a/crates/wasm/src/compile.rs +++ b/crates/wasm/src/compile.rs @@ -76,7 +76,11 @@ pub fn compile(args: JsValue) -> JsValue { // For now we default to plonk width = 3, though we can add it as a parameter let language = acvm::Language::PLONKCSat { width: 3 }; - let mut driver = noirc_driver::Driver::new(&language); + let mut driver = noirc_driver::Driver::new( + &language, + #[allow(deprecated)] + Box::new(acvm::default_is_opcode_supported(language.clone())), + ); let path = PathBuf::from(&options.entry_point); driver.create_local_crate(path, CrateType::Binary); From cfbc1f75f6f98e39a85153d36ae60950881d780a Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 18 May 2023 07:16:17 +0800 Subject: [PATCH 6/7] chore: clarify that `verify_signature` takes a hashed message (#1365) --- noir_stdlib/src/ecdsa_secp256k1.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir_stdlib/src/ecdsa_secp256k1.nr b/noir_stdlib/src/ecdsa_secp256k1.nr index b3b5135022c..18ad37bbd94 100644 --- a/noir_stdlib/src/ecdsa_secp256k1.nr +++ b/noir_stdlib/src/ecdsa_secp256k1.nr @@ -1,2 +1,2 @@ #[foreign(ecdsa_secp256k1)] -fn verify_signature(_public_key_x : [u8; 32], _public_key_y : [u8; 32], _signature: [u8; 64], _message: [u8]) -> Field {} +fn verify_signature(_public_key_x : [u8; 32], _public_key_y : [u8; 32], _signature: [u8; 64], _message_hash: [u8]) -> Field {} From ba15d6d654739cc710e147dc08d94dcfe9dedb2a Mon Sep 17 00:00:00 2001 From: guipublic <47281315+guipublic@users.noreply.github.com> Date: Thu, 18 May 2023 01:33:54 +0200 Subject: [PATCH 7/7] fix: Fix modulo operator for comptime values (#1361) use integer type for modulo simplification --- crates/nargo_cli/tests/test_data/2_div/src/main.nr | 1 + crates/noirc_evaluator/src/ssa/node.rs | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/nargo_cli/tests/test_data/2_div/src/main.nr b/crates/nargo_cli/tests/test_data/2_div/src/main.nr index 00608cb697d..ff0dee755cc 100644 --- a/crates/nargo_cli/tests/test_data/2_div/src/main.nr +++ b/crates/nargo_cli/tests/test_data/2_div/src/main.nr @@ -3,4 +3,5 @@ fn main(mut x: u32, y: u32, z: u32) { let a = x % y; assert(x / y == z); assert(a == x - z*y); + assert((50 as u64) % (9 as u64) == 5); } diff --git a/crates/noirc_evaluator/src/ssa/node.rs b/crates/noirc_evaluator/src/ssa/node.rs index 4566d974813..b964743b370 100644 --- a/crates/noirc_evaluator/src/ssa/node.rs +++ b/crates/noirc_evaluator/src/ssa/node.rs @@ -922,7 +922,10 @@ impl Binary { } else if l_is_zero { return Ok(l_eval); //TODO what is the correct result? } else if let (Some(lhs), Some(rhs)) = (lhs, rhs) { - return Ok(NodeEval::Const(lhs - rhs * (lhs / rhs), res_type)); + let lhs = res_type.field_to_type(lhs).to_u128(); + let rhs = res_type.field_to_type(rhs).to_u128(); + let result = lhs - rhs * (lhs / rhs); + return Ok(NodeEval::Const(FieldElement::from(result), res_type)); } } BinaryOp::Srem(loc) => {