From 902fbfd6fa366e4b42213ea7372ac43802ccbaaa Mon Sep 17 00:00:00 2001 From: sirasistant Date: Mon, 12 Jun 2023 18:36:20 +0000 Subject: [PATCH 1/6] feat: added cast instruction --- .../brillig_cast/Nargo.toml | 5 ++ .../brillig_cast/Prover.toml | 0 .../brillig_cast/src/main.nr | 50 +++++++++++++++++++ .../src/brillig/brillig_gen.rs | 48 +++++++++++++++++- .../noirc_evaluator/src/brillig/brillig_ir.rs | 15 ++++++ .../src/ssa_refactor/acir_gen/mod.rs | 2 +- 6 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/src/main.nr diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/Nargo.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/Prover.toml b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/Prover.toml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/src/main.nr b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/src/main.nr new file mode 100644 index 00000000000..e258a8f2640 --- /dev/null +++ b/crates/nargo_cli/tests/test_data_ssa_refactor/brillig_cast/src/main.nr @@ -0,0 +1,50 @@ +// Tests a very simple Brillig function. +// +// The features being tested are cast operations on brillig +fn main() { + bool_casts(); + field_casts(); + uint_casts(); + int_casts(); + mixed_casts(); +} + +unconstrained fn bool_casts() { + assert(false == 0 as bool); + assert(true == 1 as bool); + assert(true == 3 as bool); +} + +unconstrained fn field_casts() { + assert(5 as u8 as Field == 5); + assert(16 as u4 as Field == 0); +} + +unconstrained fn uint_casts() { + let x: u32 = 100; + assert(x as u2 == 0); + assert(x as u4 == 4); + assert(x as u6 == 36); + assert(x as u8 == 100); + assert(x as u64 == 100); + assert(x as u126 == 100); +} + +unconstrained fn int_casts() { + let x: i32 = 100; + assert(x as i2 == 0); + assert(x as i4 == 4); + assert(x as i6 == -28 as i6); + assert(x as i8 == 100); + assert(x as i8 == 100); + assert(x as i8 == 100); +} + + +unconstrained fn mixed_casts() { + assert(100 as u32 as i32 as u32 == 100); + assert(13 as u4 as i2 as u32 == 1); + assert(15 as u4 as i2 as u32 == 3); + assert(1 as u8 as bool == true); + assert(true as i8 == 1); +} diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen.rs b/crates/noirc_evaluator/src/brillig/brillig_gen.rs index a1f48606abe..6fac1299ee9 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen.rs @@ -178,10 +178,57 @@ impl BrilligGen { let source = self.convert_ssa_value(*value, dfg); self.context.truncate_instruction(destination, source); } + Instruction::Cast(value, target_type) => { + let result_ids = dfg.instruction_results(instruction_id); + let destination = self.get_or_create_register(result_ids[0]); + let source = self.convert_ssa_value(*value, dfg); + self.convert_cast(destination, source, target_type, &dfg.type_of_value(*value)); + } _ => todo!("ICE: Instruction not supported {instruction:?}"), }; } + fn convert_cast( + &mut self, + destination: RegisterIndex, + source: RegisterIndex, + target_type: &Type, + source_type: &Type, + ) { + fn numeric_to_bit_size(typ: &NumericType) -> Option { + match typ { + NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { + Some(*bit_size) + } + NumericType::NativeField => None, + } + } + + match (source_type, target_type) { + (Type::Numeric(source_numeric_type), Type::Numeric(target_numeric_type)) => { + let source_bit_size = numeric_to_bit_size(source_numeric_type); + let target_bit_size = numeric_to_bit_size(target_numeric_type); + match (source_bit_size, target_bit_size) { + (Some(source_bit_size), Some(target_bit_size)) => { + if source_bit_size > target_bit_size { + self.context.cast_instruction(destination, source, target_bit_size); + } else { + self.context.mov_instruction(destination, source); + } + } + (None, Some(target_bit_size)) => { + self.context.cast_instruction(destination, source, target_bit_size); + } + _ => { + // Casting to field is always a no_op + self.context.mov_instruction(destination, source); + } + } + } + _ => unimplemented!("The cast operation is only valid for integers."), + } + } + /// Converts the Binary instruction into a sequence of Brillig opcodes. fn convert_ssa_binary( &mut self, @@ -231,7 +278,6 @@ impl BrilligGen { brillig.convert_ssa_function(func); - brillig.context.artifact() } diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir.rs b/crates/noirc_evaluator/src/brillig/brillig_ir.rs index 9e9c0eb9a76..9be243fd5a4 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir.rs @@ -301,6 +301,21 @@ impl BrilligContext { rhs: scratch_register_j, }); } + + pub(crate) fn cast_instruction( + &mut self, + destination: RegisterIndex, + source: RegisterIndex, + target_bit_size: u32, + ) { + assert!( + target_bit_size < 127, + "tried to cast to a bit size greater than 126 {target_bit_size}" + ); + + let modulus = self.make_constant(Value::from(1_u128 << target_bit_size)); + self.modulo_instruction(destination, source, modulus, target_bit_size + 1, false); + } } /// Type to encapsulate the binary operation types in Brillig diff --git a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs index 39aabda4bb1..c6e6f992270 100644 --- a/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs +++ b/crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; -use crate::brillig::{Brillig, brillig_ir::artifact::BrilligArtifact}; +use crate::brillig::{brillig_ir::artifact::BrilligArtifact, Brillig}; use self::acir_ir::{ acir_variable::{AcirContext, AcirType, AcirVar}, From 390e9ef3feddcc13d3272e61e77810ff7a836c4e Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 13 Jun 2023 11:45:48 +0000 Subject: [PATCH 2/6] docs: add comment on cast --- crates/noirc_evaluator/src/brillig/brillig_gen.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen.rs b/crates/noirc_evaluator/src/brillig/brillig_gen.rs index 6fac1299ee9..0239541eae2 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen.rs @@ -188,6 +188,8 @@ impl BrilligGen { }; } + /// Converts an SSA cast to a sequence of Brillig opcodes. + /// Casting is only necessary when shrinking the bit size of a numeric value. fn convert_cast( &mut self, destination: RegisterIndex, From 29198d8493d85e600c8a6b2cd3675c63f5ab220e Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 13 Jun 2023 11:49:36 +0000 Subject: [PATCH 3/6] docs: added comment on cast instruction --- crates/noirc_evaluator/src/brillig/brillig_ir.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir.rs b/crates/noirc_evaluator/src/brillig/brillig_ir.rs index 9be243fd5a4..71d8c1ccc8e 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir.rs @@ -302,6 +302,7 @@ impl BrilligContext { }); } + /// Emits a modulo instruction against 2**target_bit_size pub(crate) fn cast_instruction( &mut self, destination: RegisterIndex, From 6b34890b69ca150b4b207c2be1104c02daa2d6c9 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Tue, 13 Jun 2023 20:26:09 +0000 Subject: [PATCH 4/6] simplify convert_cast --- .../src/brillig/brillig_gen.rs | 54 ++++++++++--------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/brillig_gen.rs b/crates/noirc_evaluator/src/brillig/brillig_gen.rs index 0239541eae2..05aac52d9e6 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_gen.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_gen.rs @@ -8,7 +8,10 @@ use crate::ssa_refactor::ir::{ types::{NumericType, Type}, value::{Value, ValueId}, }; -use acvm::acir::brillig_vm::{BinaryFieldOp, BinaryIntOp, RegisterIndex}; +use acvm::{ + acir::brillig_vm::{BinaryFieldOp, BinaryIntOp, RegisterIndex}, + FieldElement, +}; use iter_extended::vecmap; use std::collections::HashMap; @@ -197,37 +200,38 @@ impl BrilligGen { target_type: &Type, source_type: &Type, ) { - fn numeric_to_bit_size(typ: &NumericType) -> Option { + fn numeric_to_bit_size(typ: &NumericType) -> u32 { match typ { - NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { - Some(*bit_size) - } - NumericType::NativeField => None, + NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => *bit_size, + NumericType::NativeField => FieldElement::max_num_bits(), } } - match (source_type, target_type) { + // Casting is only valid for numeric types + // This should be checked by the frontend, so we panic if this is the case + let (source_numeric_type, target_numeric_type) = match (source_type, target_type) { (Type::Numeric(source_numeric_type), Type::Numeric(target_numeric_type)) => { - let source_bit_size = numeric_to_bit_size(source_numeric_type); - let target_bit_size = numeric_to_bit_size(target_numeric_type); - match (source_bit_size, target_bit_size) { - (Some(source_bit_size), Some(target_bit_size)) => { - if source_bit_size > target_bit_size { - self.context.cast_instruction(destination, source, target_bit_size); - } else { - self.context.mov_instruction(destination, source); - } - } - (None, Some(target_bit_size)) => { - self.context.cast_instruction(destination, source, target_bit_size); - } - _ => { - // Casting to field is always a no_op - self.context.mov_instruction(destination, source); - } - } + (source_numeric_type, target_numeric_type) } _ => unimplemented!("The cast operation is only valid for integers."), + }; + + let source_bit_size = numeric_to_bit_size(source_numeric_type); + let target_bit_size = numeric_to_bit_size(target_numeric_type); + + // Casting from a larger bit size to a smaller bit size (narrowing cast) + // requires a cast instruction. + // If its a widening cast, ie casting from a smaller bit size to a larger bit size + // we simply put a mov instruction as a no-op + // + // Field elements by construction always have the largest bit size + // This means that casting to a Field element, will always be a widening cast + // and therefore a no-op. Conversely, casting from a Field element + // will always be a narrowing cast and therefore a cast instruction + if source_bit_size > target_bit_size { + self.context.cast_instruction(destination, source, target_bit_size); + } else { + self.context.mov_instruction(destination, source); } } From 388c407ad2be2b16f40b896d09e8f1a52d096ac8 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Tue, 13 Jun 2023 20:51:25 +0000 Subject: [PATCH 5/6] Alvaro to review --- crates/noirc_evaluator/src/brillig/brillig_ir.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir.rs b/crates/noirc_evaluator/src/brillig/brillig_ir.rs index 71d8c1ccc8e..a1e47b42255 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir.rs @@ -19,6 +19,18 @@ use acvm::{ FieldElement, }; +/// Integer arithmetic in Brillig is limited to 128 bit +/// integers. +/// +/// We could lift this in the future and have Brillig +/// do big integer arithmetic when it exceeds the field size +/// or we could have users re-implement big integer arithmetic +/// in Brillig. +/// Since constrained functions do not have this property, it +/// would mean that unconstrained functions will differ from +/// constrained functions in terms of syntax compatibility. +const BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE: u32 = 128; + /// Brillig context object that is used while constructing the /// Brillig bytecode. #[derive(Default)] @@ -303,6 +315,10 @@ impl BrilligContext { } /// Emits a modulo instruction against 2**target_bit_size + /// + /// Integer arithmetic in Brillig is currently constrained to 128 bit integers. + /// We restrict the cast operation, so that integer types over 128 bits + /// cannot be created. pub(crate) fn cast_instruction( &mut self, destination: RegisterIndex, From f143358f5f890cb6ff1de1d9d063b8d36263d43a Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 14 Jun 2023 08:30:00 +0000 Subject: [PATCH 6/6] feat: do casts as no ops instead --- .../noirc_evaluator/src/brillig/brillig_ir.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/noirc_evaluator/src/brillig/brillig_ir.rs b/crates/noirc_evaluator/src/brillig/brillig_ir.rs index a1e47b42255..50784888546 100644 --- a/crates/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/crates/noirc_evaluator/src/brillig/brillig_ir.rs @@ -19,7 +19,7 @@ use acvm::{ FieldElement, }; -/// Integer arithmetic in Brillig is limited to 128 bit +/// Integer arithmetic in Brillig is limited to 127 bit /// integers. /// /// We could lift this in the future and have Brillig @@ -29,7 +29,7 @@ use acvm::{ /// Since constrained functions do not have this property, it /// would mean that unconstrained functions will differ from /// constrained functions in terms of syntax compatibility. -const BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE: u32 = 128; +const BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE: u32 = 127; /// Brillig context object that is used while constructing the /// Brillig bytecode. @@ -316,8 +316,8 @@ impl BrilligContext { /// Emits a modulo instruction against 2**target_bit_size /// - /// Integer arithmetic in Brillig is currently constrained to 128 bit integers. - /// We restrict the cast operation, so that integer types over 128 bits + /// Integer arithmetic in Brillig is currently constrained to 127 bit integers. + /// We restrict the cast operation, so that integer types over 127 bits /// cannot be created. pub(crate) fn cast_instruction( &mut self, @@ -326,12 +326,15 @@ impl BrilligContext { target_bit_size: u32, ) { assert!( - target_bit_size < 127, - "tried to cast to a bit size greater than 126 {target_bit_size}" + target_bit_size <= BRILLIG_INTEGER_ARITHMETIC_BIT_SIZE, + "tried to cast to a bit size greater than allowed {target_bit_size}" ); - let modulus = self.make_constant(Value::from(1_u128 << target_bit_size)); - self.modulo_instruction(destination, source, modulus, target_bit_size + 1, false); + // The brillig VM performs all arithmetic operations modulo 2**bit_size + // So to cast any value to a target bit size we can just issue a no-op arithmetic operation + // With bit size equal to target_bit_size + let zero = self.make_constant(Value::from(FieldElement::zero())); + self.binary_instruction(source, zero, destination, BrilligBinaryOp::Integer { op: BinaryIntOp::Add, bit_size: target_bit_size }); } }