diff --git a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs index 3456b013f52..a4eaf6bfd91 100644 --- a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs +++ b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs @@ -379,18 +379,23 @@ impl> AcirContext { return Ok(lhs); } - let bit_size = typ.bit_size::(); - if bit_size == 1 { - // Operands are booleans. - // - // a ^ b == a + b - 2*a*b - let prod = self.mul_var(lhs, rhs)?; - let sum = self.add_var(lhs, rhs)?; - self.add_mul_var(sum, -F::from(2_u128), prod) - } else { - let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; - let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs, 1)?; - Ok(outputs[0]) + match typ.to_numeric_type() { + NumericType::Signed { bit_size: 1 } | NumericType::Unsigned { bit_size: 1 } => { + // Operands are booleans. + // + // a ^ b == a + b - 2*a*b + let prod = self.mul_var(lhs, rhs)?; + let sum = self.add_var(lhs, rhs)?; + self.add_mul_var(sum, -F::from(2_u128), prod) + } + NumericType::Signed { .. } | NumericType::Unsigned { .. } => { + let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; + let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs, 1)?; + Ok(outputs[0]) + } + NumericType::NativeField => { + unreachable!("Attempted to perform bitwise operation on `Field` type") + } } } @@ -413,14 +418,19 @@ impl> AcirContext { return Ok(zero); } - let bit_size = typ.bit_size::(); - if bit_size == 1 { - // Operands are booleans. - self.mul_var(lhs, rhs) - } else { - let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; - let outputs = self.black_box_function(BlackBoxFunc::AND, inputs, 1)?; - Ok(outputs[0]) + match typ.to_numeric_type() { + NumericType::Signed { bit_size: 1 } | NumericType::Unsigned { bit_size: 1 } => { + // Operands are booleans. + self.mul_var(lhs, rhs) + } + NumericType::Signed { .. } | NumericType::Unsigned { .. } => { + let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; + let outputs = self.black_box_function(BlackBoxFunc::AND, inputs, 1)?; + Ok(outputs[0]) + } + NumericType::NativeField => { + unreachable!("Attempted to perform bitwise operation on `Field` type") + } } } @@ -441,20 +451,25 @@ impl> AcirContext { return Ok(lhs); } - let bit_size = typ.bit_size::(); - if bit_size == 1 { - // Operands are booleans - // a + b - ab - let mul = self.mul_var(lhs, rhs)?; - let sum = self.add_var(lhs, rhs)?; - self.sub_var(sum, mul) - } else { - // Implement OR in terms of AND - // (NOT a) AND (NOT b) => NOT (a OR b) - let a = self.not_var(lhs, typ.clone())?; - let b = self.not_var(rhs, typ.clone())?; - let a_and_b = self.and_var(a, b, typ.clone())?; - self.not_var(a_and_b, typ) + match typ.to_numeric_type() { + NumericType::Signed { bit_size: 1 } | NumericType::Unsigned { bit_size: 1 } => { + // Operands are booleans + // a + b - ab + let mul = self.mul_var(lhs, rhs)?; + let sum = self.add_var(lhs, rhs)?; + self.sub_var(sum, mul) + } + NumericType::Signed { .. } | NumericType::Unsigned { .. } => { + // Implement OR in terms of AND + // (NOT a) AND (NOT b) => NOT (a OR b) + let a = self.not_var(lhs, typ.clone())?; + let b = self.not_var(rhs, typ.clone())?; + let a_and_b = self.and_var(a, b, typ.clone())?; + self.not_var(a_and_b, typ) + } + NumericType::NativeField => { + unreachable!("Attempted to perform bitwise operation on `Field` type") + } } } @@ -1275,7 +1290,7 @@ impl> AcirContext { let same_sign = self.xor_var( lhs_sign, rhs_sign, - AcirType::NumericType(NumericType::Signed { bit_size: 1 }), + AcirType::NumericType(NumericType::Unsigned { bit_size: 1 }), )?; // We compute the input difference @@ -1289,7 +1304,7 @@ impl> AcirContext { self.xor_var( diff_sign, same_sign, - AcirType::NumericType(NumericType::Signed { bit_size: 1 }), + AcirType::NumericType(NumericType::Unsigned { bit_size: 1 }), ) } diff --git a/compiler/noirc_evaluator/src/ssa/validation/mod.rs b/compiler/noirc_evaluator/src/ssa/validation/mod.rs index 9171df7df94..556ccc114db 100644 --- a/compiler/noirc_evaluator/src/ssa/validation/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/validation/mod.rs @@ -13,8 +13,9 @@ //! At the moment, only [Instruction::Binary], [Instruction::ArrayGet], and [Instruction::ArraySet] //! are type checked. use core::panic; +use std::sync::Arc; -use acvm::{AcirField, FieldElement}; +use acvm::{AcirField, FieldElement, acir::BlackBoxFunc}; use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; pub(crate) mod dynamic_array_indices; @@ -148,26 +149,25 @@ impl<'f> Validator<'f> { Instruction::Binary(Binary { lhs, rhs, operator }) => { let lhs_type = dfg.type_of_value(*lhs); let rhs_type = dfg.type_of_value(*rhs); - match operator { - BinaryOp::Lt => { - if lhs_type != rhs_type { - panic!( - "Left-hand side and right-hand side of `lt` must have the same type" - ); - } - if matches!(lhs_type, Type::Numeric(NumericType::NativeField)) { - panic!("Cannot use `lt` with field elements"); - } - } - _ => { - if lhs_type != rhs_type { - panic!( - "Left-hand side and right-hand side of `{operator}` must have the same type" - ); - } - } - } + assert_eq!( + lhs_type, rhs_type, + "Left-hand side and right-hand side of `{operator}` must have the same type" + ); + + if lhs_type == Type::field() + && matches!( + operator, + BinaryOp::Lt + | BinaryOp::And + | BinaryOp::Or + | BinaryOp::Xor + | BinaryOp::Shl + | BinaryOp::Shr + ) + { + panic!("Cannot use `{operator}` with field elements"); + }; } Instruction::ArrayGet { index, .. } | Instruction::ArraySet { index, .. } => { let index_type = dfg.type_of_value(*index); @@ -203,6 +203,30 @@ impl<'f> Validator<'f> { Type::Numeric(NumericType::NativeField) )); } + Intrinsic::BlackBox(blackbox) => match blackbox { + BlackBoxFunc::AND | BlackBoxFunc::XOR => { + assert_eq!(arguments.len(), 2); + let value_typ = dfg.type_of_value(arguments[0]); + assert!( + matches!( + value_typ, + Type::Numeric( + NumericType::Unsigned { .. } + | NumericType::Signed { .. } + ) + ), + "Bitwise operation performed on non-integer type" + ); + } + BlackBoxFunc::Keccakf1600 => { + assert_eq!(arguments.len(), 1); + assert_eq!( + dfg.type_of_value(arguments[0]), + Type::Array(Arc::new(vec![Type::unsigned(64)]), 25) + ); + } + _ => {} + }, _ => {} } } @@ -752,4 +776,43 @@ mod tests { "; let _ = Ssa::from_str(src).unwrap(); } + + #[test] + #[should_panic(expected = "Cannot use `and` with field elements")] + fn bitwise_and_has_incorrect_type() { + let src = " + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v2 = and v0, v1 + return v2 + } + "; + let _ = Ssa::from_str(src).unwrap(); + } + + #[test] + #[should_panic(expected = "Cannot use `or` with field elements")] + fn bitwise_or_has_incorrect_type() { + let src = " + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v2 = or v0, v1 + return v2 + } + "; + let _ = Ssa::from_str(src).unwrap(); + } + + #[test] + #[should_panic(expected = "Cannot use `xor` with field elements")] + fn bitwise_xor_has_incorrect_type() { + let src = " + acir(inline) fn main f0 { + b0(v0: Field, v1: Field): + v2 = xor v0, v1 + return v2 + } + "; + let _ = Ssa::from_str(src).unwrap(); + } }