From 608f83c7a653db802c14b0ad0d0e299402eeb3bc Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Wed, 12 Apr 2023 12:26:13 -0400 Subject: [PATCH 1/3] initial work for wrapping addition with unsigned int --- brillig_bytecode/Cargo.toml | 1 + brillig_bytecode/src/lib.rs | 48 ++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/brillig_bytecode/Cargo.toml b/brillig_bytecode/Cargo.toml index 54746490d..3b1c9f94f 100644 --- a/brillig_bytecode/Cargo.toml +++ b/brillig_bytecode/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" [dependencies] acir_field.workspace = true serde.workspace = true +num-bigint = "0.4" [features] bn254 = ["acir_field/bn254"] diff --git a/brillig_bytecode/src/lib.rs b/brillig_bytecode/src/lib.rs index 2056b02ee..7d254d358 100644 --- a/brillig_bytecode/src/lib.rs +++ b/brillig_bytecode/src/lib.rs @@ -10,7 +10,6 @@ mod opcodes; mod registers; mod value; -use std::array; use std::collections::BTreeMap; use acir_field::FieldElement; @@ -19,6 +18,7 @@ pub use opcodes::{BinaryOp, Comparison, Opcode, OracleData}; pub use registers::{RegisterIndex, Registers}; pub use value::Typ; pub use value::Value; +use num_bigint::{BigInt, Sign}; #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum VMStatus { @@ -214,6 +214,52 @@ impl VM { let result_value = op.function()(lhs_value, rhs_value); + // match op { + // BinaryOp::Add => { + + // } + // BinaryOp::Sub => { + + // } + // BinaryOp::Mul | BinaryOp::Div => { + + // } + // } + + let wrapped_result = match result_type { + // TODO: sub just add and mul rn + Typ::Unsigned { bit_size } => { + // If result_value is greater than result_type bit size wrap around + // let leading_bits = result_value.inner.num_bits() - bit_size; + match op { + BinaryOp::Sub => { + // TODO wrap the opposite way of match below + result_value + } + _ => { + let result_as_int: u128 = result_value.inner.try_into_u128().expect("unsigned integer value cannot be larger than u128"); + if result_value.inner.num_bits() > bit_size { + let max_size: u128 = (1 << bit_size) - 1; + let new_result = result_as_int - max_size - 1; + if new_result > max_size { panic!("ICE: cannot have a binary op result be larger than its result type's bit size") } + Value { typ: Typ::Unsigned { bit_size }, inner: new_result.into() } + } else { + result_value + } + } + } + + } + Typ::Signed { bit_size } => { + // TODO possibly use BigInt here? + // let x = BigInt::from_bytes_be(Sign::Minus, &result_value.inner.to_be_bytes()); + // result_value + result_value + } + _ => result_value, + }; + + self.registers.set(result, result_value) } From e254d9c73a7330cc1c522b75fba928c3ca1b70d6 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Fri, 14 Apr 2023 10:34:46 -0400 Subject: [PATCH 2/3] new evaluate method for bin ops that checks the res_type and wraps ops appropriately --- brillig_bytecode/src/lib.rs | 50 +--------------- brillig_bytecode/src/opcodes.rs | 101 +++++++++++++++++++++++++++++--- 2 files changed, 95 insertions(+), 56 deletions(-) diff --git a/brillig_bytecode/src/lib.rs b/brillig_bytecode/src/lib.rs index 5f68ab8cb..e59ff0acb 100644 --- a/brillig_bytecode/src/lib.rs +++ b/brillig_bytecode/src/lib.rs @@ -13,12 +13,12 @@ mod value; use std::collections::BTreeMap; use acir_field::FieldElement; +use num_bigint::{BigInt, Sign}; pub use opcodes::RegisterMemIndex; pub use opcodes::{BinaryOp, Comparison, Opcode, OracleData}; pub use registers::{RegisterIndex, Registers}; pub use value::Typ; pub use value::Value; -use num_bigint::{BigInt, Sign}; #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum VMStatus { @@ -216,53 +216,7 @@ impl VM { let lhs_value = self.registers.get(lhs); let rhs_value = self.registers.get(rhs); - let result_value = op.function()(lhs_value, rhs_value); - - // match op { - // BinaryOp::Add => { - - // } - // BinaryOp::Sub => { - - // } - // BinaryOp::Mul | BinaryOp::Div => { - - // } - // } - - let wrapped_result = match result_type { - // TODO: sub just add and mul rn - Typ::Unsigned { bit_size } => { - // If result_value is greater than result_type bit size wrap around - // let leading_bits = result_value.inner.num_bits() - bit_size; - match op { - BinaryOp::Sub => { - // TODO wrap the opposite way of match below - result_value - } - _ => { - let result_as_int: u128 = result_value.inner.try_into_u128().expect("unsigned integer value cannot be larger than u128"); - if result_value.inner.num_bits() > bit_size { - let max_size: u128 = (1 << bit_size) - 1; - let new_result = result_as_int - max_size - 1; - if new_result > max_size { panic!("ICE: cannot have a binary op result be larger than its result type's bit size") } - Value { typ: Typ::Unsigned { bit_size }, inner: new_result.into() } - } else { - result_value - } - } - } - - } - Typ::Signed { bit_size } => { - // TODO possibly use BigInt here? - // let x = BigInt::from_bytes_be(Sign::Minus, &result_value.inner.to_be_bytes()); - // result_value - result_value - } - _ => result_value, - }; - + let result_value = op.evaluate(lhs_value, rhs_value, result_type); self.registers.set(result, result_value) } diff --git a/brillig_bytecode/src/opcodes.rs b/brillig_bytecode/src/opcodes.rs index 02d7df19e..5349ad94f 100644 --- a/brillig_bytecode/src/opcodes.rs +++ b/brillig_bytecode/src/opcodes.rs @@ -1,3 +1,5 @@ +use std::ops::{Add, Mul, Sub}; + use crate::{ memory::ArrayIndex, value::{Typ, Value}, @@ -134,17 +136,100 @@ pub enum Comparison { } impl BinaryOp { - pub fn function(&self) -> fn(Value, Value) -> Value { + // pub fn function(&self) -> fn(Value, Value) -> Value { + // match self { + // BinaryOp::Add => |a: Value, b: Value| a + b, + // BinaryOp::Sub => |a: Value, b: Value| a - b, + // BinaryOp::Mul => |a: Value, b: Value| a * b, + // BinaryOp::Div => |a: Value, b: Value| a / b, + // BinaryOp::Cmp(comparison) => match comparison { + // Comparison::Eq => |a: Value, b: Value| (a == b).into(), + // Comparison::Lt => |a: Value, b: Value| (a.inner < b.inner).into(), + // Comparison::Lte => |a: Value, b: Value| (a.inner <= b.inner).into(), + // }, + // } + // } + + pub fn evaluate(&self, a: Value, b: Value, res_type: Typ) -> Value { match self { - BinaryOp::Add => |a: Value, b: Value| a + b, - BinaryOp::Sub => |a: Value, b: Value| a - b, - BinaryOp::Mul => |a: Value, b: Value| a * b, - BinaryOp::Div => |a: Value, b: Value| a / b, + BinaryOp::Add => { + let res_inner = self.wrapping(a.inner, b.inner, res_type, u128::add, Add::add); + Value { typ: res_type, inner: res_inner } + } + BinaryOp::Sub => { + let res_inner = + self.wrapping(a.inner, b.inner, res_type, u128::wrapping_sub, Sub::sub); + Value { typ: res_type, inner: res_inner } + } + BinaryOp::Mul => { + let res_inner = self.wrapping(a.inner, b.inner, res_type, u128::mul, Mul::mul); + Value { typ: res_type, inner: res_inner } + } + BinaryOp::Div => match res_type { + Typ::Field => a / b, + Typ::Unsigned { bit_size } => { + let lhs = a.inner.to_u128() % (1_u128 << bit_size); + let rhs = b.inner.to_u128() % (1_u128 << bit_size); + Value { typ: res_type, inner: FieldElement::from(lhs / rhs) } + } + Typ::Signed { bit_size } => { + let a = field_to_signed(a.inner, bit_size); + let b = field_to_signed(b.inner, bit_size); + let res_inner = signed_to_field(a / b, bit_size); + Value { typ: res_type, inner: res_inner } + } + }, BinaryOp::Cmp(comparison) => match comparison { - Comparison::Eq => |a: Value, b: Value| (a == b).into(), - Comparison::Lt => |a: Value, b: Value| (a.inner < b.inner).into(), - Comparison::Lte => |a: Value, b: Value| (a.inner <= b.inner).into(), + Comparison::Eq => (a == b).into(), + Comparison::Lt => (a.inner < b.inner).into(), + Comparison::Lte => (a.inner <= b.inner).into(), }, } } + + /// Perform the given numeric operation and modulo the result by the max value for the given bit count + /// if the res_type is not a FieldElement. + fn wrapping( + &self, + lhs: FieldElement, + rhs: FieldElement, + res_type: Typ, + u128_op: impl FnOnce(u128, u128) -> u128, + field_op: impl FnOnce(FieldElement, FieldElement) -> FieldElement, + ) -> FieldElement { + match res_type { + Typ::Field => field_op(lhs, rhs), + Typ::Unsigned { bit_size } | Typ::Signed { bit_size } => { + let type_modulo = 1_u128 << bit_size; + let lhs = lhs.to_u128() % type_modulo; + let rhs = rhs.to_u128() % type_modulo; + let mut x = u128_op(lhs, rhs); + x %= type_modulo; + FieldElement::from(x) + } + } + } +} + +fn field_to_signed(f: FieldElement, n: u32) -> i128 { + assert!(n < 127); + let a = f.to_u128(); + let pow_2 = 2_u128.pow(n); + if a < pow_2 { + a as i128 + } else { + (a - 2 * pow_2) as i128 + } +} + +fn signed_to_field(a: i128, n: u32) -> FieldElement { + if n >= 126 { + panic!("ICE: cannot convert signed {n} bit size into field"); + } + if a >= 0 { + FieldElement::from(a) + } else { + let b = (a + 2_i128.pow(n + 1)) as u128; + FieldElement::from(b) + } } From b3e1d45e8f6d3be1f6dcb13358553708a249efd0 Mon Sep 17 00:00:00 2001 From: Maxim Vezenov Date: Mon, 17 Apr 2023 16:51:11 -0400 Subject: [PATCH 3/3] handle signed and unsigned division for binary ops --- brillig_bytecode/src/lib.rs | 2 +- brillig_bytecode/src/opcodes.rs | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/brillig_bytecode/src/lib.rs b/brillig_bytecode/src/lib.rs index a4b8e2581..879e4e1dd 100644 --- a/brillig_bytecode/src/lib.rs +++ b/brillig_bytecode/src/lib.rs @@ -211,7 +211,7 @@ impl VM { lhs: RegisterMemIndex, rhs: RegisterMemIndex, result: RegisterIndex, - _result_type: Typ, + result_type: Typ, ) { let lhs_value = self.registers.get(lhs); let rhs_value = self.registers.get(rhs); diff --git a/brillig_bytecode/src/opcodes.rs b/brillig_bytecode/src/opcodes.rs index cf90806ec..0982d3ac9 100644 --- a/brillig_bytecode/src/opcodes.rs +++ b/brillig_bytecode/src/opcodes.rs @@ -142,20 +142,6 @@ pub enum Comparison { } impl BinaryOp { - // pub fn function(&self) -> fn(Value, Value) -> Value { - // match self { - // BinaryOp::Add => |a: Value, b: Value| a + b, - // BinaryOp::Sub => |a: Value, b: Value| a - b, - // BinaryOp::Mul => |a: Value, b: Value| a * b, - // BinaryOp::Div => |a: Value, b: Value| a / b, - // BinaryOp::Cmp(comparison) => match comparison { - // Comparison::Eq => |a: Value, b: Value| (a == b).into(), - // Comparison::Lt => |a: Value, b: Value| (a.inner < b.inner).into(), - // Comparison::Lte => |a: Value, b: Value| (a.inner <= b.inner).into(), - // }, - // } - // } - pub fn evaluate(&self, a: Value, b: Value, res_type: Typ) -> Value { match self { BinaryOp::Add => {