From 52b6f9cc6aff1f8798ae824fb720e8ac4d53ff89 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 23 Oct 2025 08:51:17 -0300 Subject: [PATCH 1/5] Let AcirValue::Var hold a NumericType --- .../src/acir/acir_context/black_box.rs | 9 ++- .../src/acir/acir_context/brillig_call.rs | 25 +++---- .../src/acir/acir_context/mod.rs | 70 +++++++++---------- compiler/noirc_evaluator/src/acir/arrays.rs | 24 +++---- .../src/acir/call/intrinsics/mod.rs | 34 +++++---- .../src/acir/call/intrinsics/slice_ops.rs | 15 ++-- compiler/noirc_evaluator/src/acir/call/mod.rs | 7 +- compiler/noirc_evaluator/src/acir/mod.rs | 34 ++++----- compiler/noirc_evaluator/src/acir/types.rs | 29 +------- .../src/ssa/interpreter/mod.rs | 2 +- .../src/ssa/ir/dfg/simplify.rs | 2 +- .../src/ssa/ir/dfg/simplify/binary.rs | 4 +- compiler/noirc_evaluator/src/ssa/ir/types.rs | 10 ++- .../src/ssa/opt/loop_invariant.rs | 3 +- .../src/ssa/opt/remove_bit_shifts.rs | 10 +-- 15 files changed, 129 insertions(+), 149 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/acir_context/black_box.rs b/compiler/noirc_evaluator/src/acir/acir_context/black_box.rs index fe04e08e0b5..568b365652e 100644 --- a/compiler/noirc_evaluator/src/acir/acir_context/black_box.rs +++ b/compiler/noirc_evaluator/src/acir/acir_context/black_box.rs @@ -1,9 +1,12 @@ use acvm::acir::{AcirField, BlackBoxFunc, circuit::opcodes::FunctionInput}; use iter_extended::vecmap; -use crate::errors::{InternalError, RuntimeError}; +use crate::{ + errors::{InternalError, RuntimeError}, + ssa::ir::types::NumericType, +}; -use super::{AcirContext, AcirType, AcirValue, AcirVar}; +use super::{AcirContext, AcirValue, AcirVar}; impl AcirContext { /// Calls a Blackbox function on the given inputs and returns a given set of outputs @@ -60,7 +63,7 @@ impl AcirContext { })); } }; - inputs.push(AcirValue::Var(predicate.unwrap(), AcirType::unsigned(1))); + inputs.push(AcirValue::Var(predicate.unwrap(), NumericType::bool())); vec![proof_type_constant] } _ => Vec::new(), diff --git a/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs b/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs index eb7c1e9caab..f1d4532873f 100644 --- a/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs +++ b/compiler/noirc_evaluator/src/acir/acir_context/brillig_call.rs @@ -49,9 +49,9 @@ impl AcirContext { // We can then immediately zero out all of its outputs as this is the value which would be written // if we waited until runtime to resolve this call. let outputs_var = vecmap(outputs, |output| match output { - AcirType::NumericType(_) => { + AcirType::NumericType(numeric_type) => { let var = self.add_constant(F::zero()); - AcirValue::Var(var, output.clone()) + AcirValue::Var(var, numeric_type) } AcirType::Array(element_types, size) => { self.zeroed_array_output(&element_types, size) @@ -84,12 +84,12 @@ impl AcirContext { let mut brillig_outputs = Vec::new(); let outputs_var = vecmap(outputs, |output| match output { - AcirType::NumericType(_) => { + AcirType::NumericType(numeric_type) => { let var = self.add_variable(); let witness_index = self.var_to_witness(var).expect("variable has just been created as witness"); brillig_outputs.push(BrilligOutputs::Simple(witness_index)); - AcirValue::Var(var, output.clone()) + AcirValue::Var(var, numeric_type) } AcirType::Array(element_types, size) => { let (acir_value, witnesses) = self.brillig_array_output(&element_types, size); @@ -113,11 +113,7 @@ impl AcirContext { ) -> Result<(), RuntimeError> { let one = context.add_constant(G::one()); match value { - AcirValue::Var(var, typ) => { - let numeric_type = match typ { - AcirType::NumericType(numeric_type) => numeric_type, - _ => unreachable!("`AcirValue::Var` may only hold primitive values"), - }; + AcirValue::Var(var, numeric_type) => { // Predicate is one so that the constrain is always applied, because // values returned from Brillig will be 0 under a false predicate. context.range_constrain_var(*var, numeric_type, None, one)?; @@ -165,8 +161,7 @@ impl AcirContext { let value_read_var = self.read_from_memory(block_id, &index_var)?; let value_typ = value_types[i % value_types.len()]; - let value_read = - AcirValue::Var(value_read_var, AcirType::NumericType(value_typ)); + let value_read = AcirValue::Var(value_read_var, value_typ); self.brillig_array_input(var_expressions, value_read)?; } @@ -186,9 +181,9 @@ impl AcirContext { self.zeroed_array_output(nested_element_types, *nested_size); array_values.push_back(nested_acir_value); } - AcirType::NumericType(_) => { + AcirType::NumericType(numeric_type) => { let var = self.add_constant(F::zero()); - array_values.push_back(AcirValue::Var(var, element_type.clone())); + array_values.push_back(AcirValue::Var(var, *numeric_type)); } } } @@ -214,9 +209,9 @@ impl AcirContext { witnesses.append(&mut nested_witnesses); array_values.push_back(nested_acir_value); } - AcirType::NumericType(_) => { + AcirType::NumericType(numeric_type) => { let var = self.add_variable(); - array_values.push_back(AcirValue::Var(var, element_type.clone())); + array_values.push_back(AcirValue::Var(var, *numeric_type)); witnesses.push( self.var_to_witness(var) .expect("variable has just been created as witness"), diff --git a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs index 69519f19c9b..6bf2936be8a 100644 --- a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs +++ b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs @@ -280,7 +280,7 @@ impl AcirContext { let results = self.stdlib_brillig_call( predicate, BrilligStdlibFunc::Inverse, - vec![AcirValue::Var(var, AcirType::field())], + vec![AcirValue::Var(var, NumericType::NativeField)], vec![AcirType::field()], )?; Self::expect_one_var(results) @@ -334,7 +334,7 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, - typ: AcirType, + typ: NumericType, ) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -351,7 +351,7 @@ impl AcirContext { return Ok(lhs); } - match typ.to_numeric_type() { + match typ { NumericType::Signed { bit_size: 1 } | NumericType::Unsigned { bit_size: 1 } => { // Operands are booleans. // @@ -361,7 +361,7 @@ impl AcirContext { self.add_mul_var(sum, -F::from(2_u128), prod) } NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { - let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; + let inputs = vec![AcirValue::Var(lhs, typ), AcirValue::Var(rhs, typ)]; let outputs = self.black_box_function(BlackBoxFunc::XOR, inputs, Some(bit_size), 1, None)?; Ok(outputs[0]) @@ -377,7 +377,7 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, - typ: AcirType, + typ: NumericType, ) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -391,13 +391,13 @@ impl AcirContext { return Ok(zero); } - match typ.to_numeric_type() { + match typ { NumericType::Signed { bit_size: 1 } | NumericType::Unsigned { bit_size: 1 } => { // Operands are booleans. self.mul_var(lhs, rhs) } NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { - let inputs = vec![AcirValue::Var(lhs, typ.clone()), AcirValue::Var(rhs, typ)]; + let inputs = vec![AcirValue::Var(lhs, typ), AcirValue::Var(rhs, typ)]; let outputs = self.black_box_function(BlackBoxFunc::AND, inputs, Some(bit_size), 1, None)?; Ok(outputs[0]) @@ -413,7 +413,7 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, - typ: AcirType, + typ: NumericType, ) -> Result { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -425,7 +425,7 @@ impl AcirContext { return Ok(lhs); } - match typ.to_numeric_type() { + match typ { NumericType::Signed { bit_size: 1 } | NumericType::Unsigned { bit_size: 1 } => { // Operands are booleans // a + b - ab @@ -436,9 +436,9 @@ impl AcirContext { 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())?; + let a = self.not_var(lhs, typ)?; + let b = self.not_var(rhs, typ)?; + let a_and_b = self.and_var(a, b, typ)?; self.not_var(a_and_b, typ) } NumericType::NativeField => { @@ -563,25 +563,22 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, - typ: AcirType, + typ: NumericType, predicate: AcirVar, ) -> Result { match typ { - AcirType::NumericType(NumericType::NativeField) => { + NumericType::NativeField => { let inv_rhs = self.inv_var(rhs, predicate)?; self.mul_var(lhs, inv_rhs) } - AcirType::NumericType(NumericType::Unsigned { bit_size }) => { + NumericType::Unsigned { bit_size } => { let (quotient_var, _remainder_var) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(quotient_var) } - AcirType::NumericType(NumericType::Signed { .. }) => { + NumericType::Signed { .. } => { unreachable!("Signed division should have been removed before ACIRgen") } - AcirType::Array(_, _) => { - unreachable!("cannot divide arrays. This should have been caught by the frontend") - } } } @@ -746,7 +743,11 @@ impl AcirContext { } /// Adds a new variable that is constrained to be the logical NOT of `x`. - pub(crate) fn not_var(&mut self, x: AcirVar, typ: AcirType) -> Result { + pub(crate) fn not_var( + &mut self, + x: AcirVar, + typ: NumericType, + ) -> Result { let bit_size = typ.bit_size::(); // Subtracting from max flips the bits let max = power_of_two::(bit_size) - F::one(); @@ -862,8 +863,8 @@ impl AcirContext { predicate, BrilligStdlibFunc::Quotient, vec![ - AcirValue::Var(lhs, AcirType::unsigned(bit_size)), - AcirValue::Var(rhs, AcirType::unsigned(bit_size)), + AcirValue::Var(lhs, NumericType::unsigned(bit_size)), + AcirValue::Var(rhs, NumericType::unsigned(bit_size)), ], vec![AcirType::unsigned(max_q_bits), AcirType::unsigned(max_rhs_bits)], )? @@ -1068,25 +1069,22 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, - typ: AcirType, + typ: NumericType, bit_size: u32, predicate: AcirVar, ) -> Result { match typ { - AcirType::NumericType(NumericType::Unsigned { .. }) => { + NumericType::Unsigned { .. } => { let (_, remainder_var) = self.euclidean_division_var(lhs, rhs, bit_size, predicate)?; Ok(remainder_var) } - AcirType::NumericType(NumericType::Signed { .. }) => { + NumericType::Signed { .. } => { unreachable!("Signed modulo should have been removed before ACIRgen") } - AcirType::NumericType(NumericType::NativeField) => { + NumericType::NativeField => { unreachable!("cannot module fields. This should have been caught by the frontend") } - AcirType::Array(_, _) => { - unreachable!("cannot modulo arrays. This should have been caught by the frontend") - } } } @@ -1245,7 +1243,7 @@ impl AcirContext { input_var: AcirVar, radix_var: AcirVar, limb_count: u32, - result_element_type: AcirType, + result_element_type: NumericType, ) -> Result { let radix = match self.vars[&radix_var].as_constant() { Some(radix) => radix.to_u128() as u32, @@ -1264,7 +1262,7 @@ impl AcirContext { let mut limb_vars = vecmap(limbs, |witness| { let witness = self.add_data(AcirVarData::Witness(witness)); - AcirValue::Var(witness, result_element_type.clone()) + AcirValue::Var(witness, result_element_type) }); if endian == Endian::Big { @@ -1280,7 +1278,7 @@ impl AcirContext { endian: Endian, input_var: AcirVar, limb_count: u32, - result_element_type: AcirType, + result_element_type: NumericType, ) -> Result { let two_var = self.add_constant(2_u128); self.radix_decompose(endian, input_var, two_var, limb_count, result_element_type) @@ -1292,7 +1290,7 @@ impl AcirContext { pub(crate) fn flatten( &mut self, value: AcirValue, - ) -> Result, InternalError> { + ) -> Result, InternalError> { match value { AcirValue::Var(acir_var, typ) => Ok(vec![(acir_var, typ)]), AcirValue::Array(array) => { @@ -1306,9 +1304,9 @@ impl AcirContext { try_vecmap(0..len, |i| { let index_var = self.add_constant(i); - Ok::<(AcirVar, AcirType), InternalError>(( + Ok::<(AcirVar, NumericType), InternalError>(( self.read_from_memory(block_id, &index_var)?, - value_types[i % value_types.len()].into(), + value_types[i % value_types.len()], )) }) } @@ -1443,7 +1441,7 @@ impl AcirContext { let index_var = self.add_constant(i); let read = self.read_from_memory(block_id, &index_var)?; let typ = value_types[i % value_types.len()]; - Ok::(AcirValue::Var(read, AcirType::NumericType(typ))) + Ok::(AcirValue::Var(read, typ)) })?; for value in dynamic_array_values { self.initialize_array_inner(witnesses, value)?; diff --git a/compiler/noirc_evaluator/src/acir/arrays.rs b/compiler/noirc_evaluator/src/acir/arrays.rs index 80070fae183..f79a286586f 100644 --- a/compiler/noirc_evaluator/src/acir/arrays.rs +++ b/compiler/noirc_evaluator/src/acir/arrays.rs @@ -132,7 +132,7 @@ use crate::ssa::ir::{ use super::{ AcirVar, Context, - types::{AcirDynamicArray, AcirType, AcirValue}, + types::{AcirDynamicArray, AcirValue}, }; impl Context<'_> { @@ -481,7 +481,7 @@ impl Context<'_> { let false_pred = self.acir_context.mul_var(not_pred, *dummy_var)?; // predicate*value + (1-predicate)*dummy let new_value = self.acir_context.add_var(true_pred, false_pred)?; - Ok(AcirValue::Var(new_value, typ.clone())) + Ok(AcirValue::Var(new_value, *typ)) } (AcirValue::Array(values), AcirValue::Array(dummy_values)) => { let mut elements = im::Vector::new(); @@ -612,10 +612,8 @@ impl Context<'_> { ) -> Result { if let AcirValue::Var(value_var, typ) = &value { let array_typ = dfg.type_of_value(array); - if let (Type::Numeric(numeric_type), AcirType::NumericType(num)) = - (array_typ.first(), typ) - { - if numeric_type.bit_size() <= num.bit_size() { + if let Type::Numeric(numeric_type) = array_typ.first() { + if numeric_type.bit_size::() <= typ.bit_size::() { // first element is compatible index_side_effect = false; } @@ -624,7 +622,7 @@ impl Context<'_> { if index_side_effect { value = AcirValue::Var( self.acir_context.mul_var(*value_var, self.current_side_effects_enabled_var)?, - typ.clone(), + *typ, ); } } @@ -646,8 +644,7 @@ impl Context<'_> { // Increment the var_index in case of a nested array *var_index = self.acir_context.add_var(*var_index, one)?; - let typ = AcirType::NumericType(numeric_type); - Ok(AcirValue::Var(read, typ)) + Ok(AcirValue::Var(read, numeric_type)) } Type::Array(element_types, len) => { let mut values = im::Vector::new(); @@ -673,8 +670,7 @@ impl Context<'_> { match ssa_type.clone() { Type::Numeric(numeric_type) => { let zero = self.acir_context.add_constant(FieldElement::zero()); - let typ = AcirType::NumericType(numeric_type); - Ok(AcirValue::Var(zero, typ)) + Ok(AcirValue::Var(zero, numeric_type)) } Type::Array(element_types, len) => { let mut values = im::Vector::new(); @@ -781,7 +777,7 @@ impl Context<'_> { let read = self.acir_context.read_from_memory(*inner_block_id, &index_var)?; let typ = value_types[i % value_types.len()]; - Ok::(AcirValue::Var(read, AcirType::NumericType(typ))) + Ok::(AcirValue::Var(read, typ)) })?; self.array_set_value(&AcirValue::Array(values.into()), block_id, var_index)?; } @@ -884,7 +880,7 @@ impl Context<'_> { // The final array should will the flattened index at each outer array index let init_values = vecmap(flat_elem_type_sizes, |type_size| { let var = self.acir_context.add_constant(type_size); - AcirValue::Var(var, AcirType::field()) + AcirValue::Var(var, NumericType::NativeField) }); let element_type_sizes_len = init_values.len(); self.initialize_array( @@ -975,7 +971,7 @@ impl Context<'_> { let read = self.acir_context.read_from_memory(source, &index_var)?; let typ = value_types[i % value_types.len()]; - Ok::(AcirValue::Var(read, AcirType::NumericType(typ))) + Ok::(AcirValue::Var(read, typ)) })?; Ok(init_values.into()) } diff --git a/compiler/noirc_evaluator/src/acir/call/intrinsics/mod.rs b/compiler/noirc_evaluator/src/acir/call/intrinsics/mod.rs index 61c9dbf5dc3..16e69980058 100644 --- a/compiler/noirc_evaluator/src/acir/call/intrinsics/mod.rs +++ b/compiler/noirc_evaluator/src/acir/call/intrinsics/mod.rs @@ -1,9 +1,5 @@ use iter_extended::vecmap; -use crate::acir::{ - arrays, - types::{AcirType, AcirValue}, -}; use crate::errors::RuntimeError; use crate::ssa::ir::{ dfg::DataFlowGraph, @@ -11,6 +7,10 @@ use crate::ssa::ir::{ types::Type, value::ValueId, }; +use crate::{ + acir::{arrays, types::AcirValue}, + ssa::ir::types::NumericType, +}; use super::Context; @@ -69,15 +69,16 @@ impl Context<'_> { else { unreachable!("ICE: ToRadix result must be an array"); }; + assert!( + result_type.len() == 1, + "ICE: ToRadix result type must have a single element type" + ); + let Type::Numeric(numeric_type) = result_type[0] else { + unreachable!("ICE: ToRadix result element type must be numeric"); + }; self.acir_context - .radix_decompose( - endian, - field, - radix, - array_length, - result_type[0].clone().into(), - ) + .radix_decompose(endian, field, radix, array_length, numeric_type) .map(|array| vec![array]) } Intrinsic::ToBits(endian) => { @@ -87,9 +88,16 @@ impl Context<'_> { else { unreachable!("ICE: ToBits result must be an array"); }; + assert!( + result_type.len() == 1, + "ICE: ToBits result type must have a single element type" + ); + let Type::Numeric(numeric_type) = result_type[0] else { + unreachable!("ICE: ToBits result element type must be numeric"); + }; self.acir_context - .bit_decompose(endian, field, array_length, result_type[0].clone().into()) + .bit_decompose(endian, field, array_length, numeric_type) .map(|array| vec![array]) } Intrinsic::AsSlice => { @@ -103,7 +111,7 @@ impl Context<'_> { let acir_value = self.convert_value(array_contents, dfg); let result = self.read_array(acir_value)?; Ok(vec![ - AcirValue::Var(slice_length, AcirType::unsigned(32)), + AcirValue::Var(slice_length, NumericType::length_type()), AcirValue::Array(result), ]) } diff --git a/compiler/noirc_evaluator/src/acir/call/intrinsics/slice_ops.rs b/compiler/noirc_evaluator/src/acir/call/intrinsics/slice_ops.rs index 6996f1e8988..8b3c50e79b6 100644 --- a/compiler/noirc_evaluator/src/acir/call/intrinsics/slice_ops.rs +++ b/compiler/noirc_evaluator/src/acir/call/intrinsics/slice_ops.rs @@ -1,6 +1,7 @@ use crate::acir::types::flat_numeric_types; -use crate::acir::{AcirDynamicArray, AcirType, AcirValue}; +use crate::acir::{AcirDynamicArray, AcirValue}; use crate::errors::RuntimeError; +use crate::ssa::ir::types::NumericType; use crate::ssa::ir::{dfg::DataFlowGraph, value::ValueId}; use acvm::{AcirField, FieldElement}; @@ -44,7 +45,7 @@ impl Context<'_> { let new_slice_val = AcirValue::Array(new_slice); - Ok(vec![AcirValue::Var(new_slice_length, AcirType::unsigned(32)), new_slice_val]) + Ok(vec![AcirValue::Var(new_slice_length, NumericType::length_type()), new_slice_val]) } /// Pushes one or more elements to the front of a non-nested slice. @@ -84,7 +85,7 @@ impl Context<'_> { let new_slice_val = AcirValue::Array(new_slice); - Ok(vec![AcirValue::Var(new_slice_length, AcirType::unsigned(32)), new_slice_val]) + Ok(vec![AcirValue::Var(new_slice_length, NumericType::length_type()), new_slice_val]) } /// Removes and returns one or more elements from the back of a non-nested slice. @@ -143,7 +144,7 @@ impl Context<'_> { let new_slice = self.read_array(slice)?; let mut results = vec![ - AcirValue::Var(new_slice_length, AcirType::unsigned(32)), + AcirValue::Var(new_slice_length, NumericType::length_type()), AcirValue::Array(new_slice), ]; results.append(&mut popped_elements); @@ -226,7 +227,7 @@ impl Context<'_> { new_slice = new_slice.slice(popped_elements_size..); - popped_elements.push(AcirValue::Var(new_slice_length, AcirType::unsigned(32))); + popped_elements.push(AcirValue::Var(new_slice_length, NumericType::length_type())); popped_elements.push(AcirValue::Array(new_slice)); Ok(popped_elements) @@ -414,7 +415,7 @@ impl Context<'_> { element_type_sizes, }); - Ok(vec![AcirValue::Var(new_slice_length, AcirType::unsigned(32)), result]) + Ok(vec![AcirValue::Var(new_slice_length, NumericType::length_type()), result]) } /// Removes one or more elements from a slice at a given index. @@ -559,7 +560,7 @@ impl Context<'_> { element_type_sizes, }); - let mut result = vec![AcirValue::Var(new_slice_length, AcirType::unsigned(32)), result]; + let mut result = vec![AcirValue::Var(new_slice_length, NumericType::length_type()), result]; result.append(&mut popped_elements); Ok(result) diff --git a/compiler/noirc_evaluator/src/acir/call/mod.rs b/compiler/noirc_evaluator/src/acir/call/mod.rs index 2cfdc2e7433..e45b761c454 100644 --- a/compiler/noirc_evaluator/src/acir/call/mod.rs +++ b/compiler/noirc_evaluator/src/acir/call/mod.rs @@ -294,9 +294,12 @@ impl Context<'_> { } AcirValue::Array(element_values) } - typ => { + Type::Numeric(numeric_type) => { let var = vars.next().unwrap(); - AcirValue::Var(var, typ.into()) + AcirValue::Var(var, *numeric_type) + } + typ => { + panic!("Unexpected type {typ} in convert_var_type_to_values"); } } } diff --git a/compiler/noirc_evaluator/src/acir/mod.rs b/compiler/noirc_evaluator/src/acir/mod.rs index 0ec1317b264..2d03be6bf81 100644 --- a/compiler/noirc_evaluator/src/acir/mod.rs +++ b/compiler/noirc_evaluator/src/acir/mod.rs @@ -363,8 +363,7 @@ impl<'a> Context<'a> { ) -> Result { match param_type { Type::Numeric(numeric_type) => { - let typ = AcirType::new(*numeric_type); - Ok(AcirValue::Var(make_var(self, *numeric_type)?, typ)) + Ok(AcirValue::Var(make_var(self, *numeric_type)?, *numeric_type)) } Type::Array(element_types, length) => { let mut elements = im::Vector::new(); @@ -557,7 +556,7 @@ impl<'a> Context<'a> { result: AcirVar, ) { let [result_id] = dfg.instruction_result(instruction); - let typ = dfg.type_of_value(result_id).into(); + let typ = dfg.type_of_value(result_id).unwrap_numeric(); self.define_result(dfg, instruction, AcirValue::Var(result, typ)); } @@ -642,8 +641,7 @@ impl<'a> Context<'a> { let acir_value = match value { Value::NumericConstant { constant, typ } => { - let typ = AcirType::from(Type::Numeric(*typ)); - AcirValue::Var(self.acir_context.add_constant(*constant), typ) + AcirValue::Var(self.acir_context.add_constant(*constant), *typ) } Value::Intrinsic(..) => { unreachable!("ICE: Intrinsics should be resolved via separate logic") @@ -653,7 +651,7 @@ impl<'a> Context<'a> { // debugging instrumentation code to work. Taking the reference // of a function in ACIR is useless. let id = self.acir_context.add_constant(function_id.to_u32()); - AcirValue::Var(id, AcirType::field()) + AcirValue::Var(id, NumericType::NativeField) } Value::ForeignFunction(_) => unimplemented!( "Oracle calls directly in constrained functions are not yet available." @@ -698,9 +696,9 @@ impl<'a> Context<'a> { ) -> Result { let lhs = self.convert_numeric_value(binary.lhs, dfg)?; let rhs = self.convert_numeric_value(binary.rhs, dfg)?; - let binary_type = self.type_of_binary_operation(binary, dfg); + let num_type = self.type_of_binary_operation(binary, dfg).unwrap_numeric(); - if binary_type.is_signed() + if num_type.is_signed() && matches!( binary.operator, BinaryOp::Add { unchecked: false } @@ -711,29 +709,25 @@ impl<'a> Context<'a> { panic!("Checked signed operations should all be removed before ACIRgen") } - let binary_type = AcirType::from(binary_type); - let bit_count = binary_type.bit_size::(); - let num_type = binary_type.to_numeric_type(); + let bit_count = num_type.bit_size::(); let result = match binary.operator { BinaryOp::Add { .. } => self.acir_context.add_var(lhs, rhs), BinaryOp::Sub { .. } => self.acir_context.sub_var(lhs, rhs), BinaryOp::Mul { .. } => self.acir_context.mul_var(lhs, rhs), - BinaryOp::Div => self.acir_context.div_var(lhs, rhs, binary_type.clone(), predicate), + BinaryOp::Div => self.acir_context.div_var(lhs, rhs, num_type, predicate), // Note: that this produces unnecessary constraints when // this Eq instruction is being used for a constrain statement BinaryOp::Eq => self.acir_context.eq_var(lhs, rhs), - BinaryOp::Lt => match binary_type { - AcirType::NumericType(NumericType::Signed { .. }) => { + BinaryOp::Lt => match num_type { + NumericType::Signed { .. } => { panic!("ICE - signed less than should have been removed before ACIRgen") } _ => self.acir_context.less_than_var(lhs, rhs, bit_count), }, - BinaryOp::Xor => self.acir_context.xor_var(lhs, rhs, binary_type), - BinaryOp::And => self.acir_context.and_var(lhs, rhs, binary_type), - BinaryOp::Or => self.acir_context.or_var(lhs, rhs, binary_type), - BinaryOp::Mod => { - self.acir_context.modulo_var(lhs, rhs, binary_type.clone(), bit_count, predicate) - } + BinaryOp::Xor => self.acir_context.xor_var(lhs, rhs, num_type), + BinaryOp::And => self.acir_context.and_var(lhs, rhs, num_type), + BinaryOp::Or => self.acir_context.or_var(lhs, rhs, num_type), + BinaryOp::Mod => self.acir_context.modulo_var(lhs, rhs, num_type, bit_count, predicate), BinaryOp::Shl | BinaryOp::Shr => unreachable!( "ICE - bit shift operators do not exist in ACIR and should have been replaced" ), diff --git a/compiler/noirc_evaluator/src/acir/types.rs b/compiler/noirc_evaluator/src/acir/types.rs index 2b2a13b38cf..2d1ee4547ad 100644 --- a/compiler/noirc_evaluator/src/acir/types.rs +++ b/compiler/noirc_evaluator/src/acir/types.rs @@ -1,6 +1,6 @@ use std::fmt::Debug; -use acvm::{AcirField, acir::circuit::opcodes::BlockId}; +use acvm::acir::circuit::opcodes::BlockId; use crate::{ errors::InternalError, @@ -23,22 +23,6 @@ pub(crate) enum AcirType { } impl AcirType { - pub(crate) fn new(typ: NumericType) -> Self { - Self::NumericType(typ) - } - - /// Returns the bit size of the underlying type - pub(crate) fn bit_size(&self) -> u32 { - match self { - AcirType::NumericType(numeric_type) => match numeric_type { - NumericType::Signed { bit_size } => *bit_size, - NumericType::Unsigned { bit_size } => *bit_size, - NumericType::NativeField => F::max_num_bits(), - }, - AcirType::Array(_, _) => unreachable!("cannot fetch bit size of array type"), - } - } - /// Returns a field type pub(crate) fn field() -> Self { AcirType::NumericType(NumericType::NativeField) @@ -48,13 +32,6 @@ impl AcirType { pub(crate) fn unsigned(bit_size: u32) -> Self { AcirType::NumericType(NumericType::Unsigned { bit_size }) } - - pub(crate) fn to_numeric_type(&self) -> NumericType { - match self { - AcirType::NumericType(numeric_type) => *numeric_type, - AcirType::Array(_, _) => unreachable!("cannot fetch a numeric type for an array type"), - } - } } impl From for AcirType { @@ -124,7 +101,7 @@ impl Debug for AcirDynamicArray { #[derive(Debug, Clone)] pub(crate) enum AcirValue { - Var(AcirVar, AcirType), + Var(AcirVar, NumericType), Array(im::Vector), DynamicArray(AcirDynamicArray), } @@ -157,7 +134,7 @@ impl AcirValue { /// This is because an [AcirValue::DynamicArray] is simply a pointer to an array /// and fetching its internal [AcirValue::Var] would require laying down opcodes to read its content. /// This method should only be used where dynamic arrays are not a possible type. - pub(super) fn flatten(self) -> Vec<(AcirVar, AcirType)> { + pub(super) fn flatten(self) -> Vec<(AcirVar, NumericType)> { match self { AcirValue::Var(var, typ) => vec![(var, typ)], AcirValue::Array(array) => array.into_iter().flat_map(AcirValue::flatten).collect(), diff --git a/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs b/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs index 60468f6a403..1b2d52b7d7c 100644 --- a/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/interpreter/mod.rs @@ -635,7 +635,7 @@ impl<'ssa, W: Write> Interpreter<'ssa, W> { fn interpret_not(&mut self, id: ValueId, result: ValueId) -> IResult<()> { let num_value = self.lookup_numeric(id, "not instruction")?; - let bit_size = num_value.get_type().bit_size(); + let bit_size = num_value.get_type().bit_size::(); // Based on AcirContext::not_var fn fitted_not>(value: Fitted, bit_size: u32) -> Fitted { diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs index b7a4c9a9e70..30f17d053a4 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify.rs @@ -85,7 +85,7 @@ pub(crate) fn simplify( // would be incorrect however since the extra bits on the field would not be flipped. Value::NumericConstant { constant, typ } if typ.is_unsigned() => { // As we're casting to a `u128`, we need to clear out any upper bits that the NOT fills. - let bit_size = typ.bit_size(); + let bit_size = typ.bit_size::(); assert!(bit_size <= 128); let not_value: u128 = truncate(!constant.to_u128(), bit_size); SimplifiedTo(dfg.make_constant(not_value.into(), *typ)) diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs index c15a0e1f243..ae9114f2a4d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/binary.rs @@ -172,7 +172,7 @@ pub(super) fn simplify_binary( return SimplifyResult::SimplifiedToInstruction(Instruction::Truncate { value: lhs, bit_size, - max_bit_size: lhs_type.bit_size(), + max_bit_size: lhs_type.bit_size::(), }); } } @@ -260,7 +260,7 @@ pub(super) fn simplify_binary( let value = if lhs_value.is_some() { rhs } else { lhs }; let bit_size = if bitmask == u128::MAX { 128 } else { (bitmask + 1).ilog2() }; - let max_bit_size = lhs_type.bit_size(); + let max_bit_size = lhs_type.bit_size::(); if bit_size == max_bit_size { // If we're truncating a value into the full size of its type then diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index 378567cea46..a39e259bb93 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -23,9 +23,9 @@ pub enum NumericType { impl NumericType { /// Returns the bit size of the provided numeric type. - pub(crate) fn bit_size(self: &NumericType) -> u32 { + pub(crate) fn bit_size(self: &NumericType) -> u32 { match self { - NumericType::NativeField => FieldElement::max_num_bits(), + NumericType::NativeField => F::max_num_bits(), NumericType::Unsigned { bit_size } | NumericType::Signed { bit_size } => *bit_size, } } @@ -85,6 +85,10 @@ impl NumericType { matches!(self, NumericType::Unsigned { .. }) } + pub(crate) fn is_signed(&self) -> bool { + matches!(self, NumericType::Signed { .. }) + } + pub(crate) fn max_value(&self) -> Result { match self { NumericType::Unsigned { bit_size } => match bit_size { @@ -211,7 +215,7 @@ impl Type { /// Panics if `self` is not a [`Type::Numeric`] pub(crate) fn bit_size(&self) -> u32 { match self { - Type::Numeric(numeric_type) => numeric_type.bit_size(), + Type::Numeric(numeric_type) => numeric_type.bit_size::(), other => panic!("bit_size: Expected numeric type, found {other}"), } } diff --git a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index d6206ff3575..195c206629e 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -852,7 +852,8 @@ fn can_be_hoisted(instruction: &Instruction, dfg: &DataFlowGraph) -> CanBeHoiste // A cast may have dependence on a range-check, which may not be hoisted, so we cannot always hoist a cast. // We can safely hoist a cast from a smaller to a larger type as no range check is necessary in this case. let source_type = dfg.type_of_value(*source).unwrap_numeric(); - (source_type.bit_size() <= target_type.bit_size()).into() + (source_type.bit_size::() <= target_type.bit_size::()) + .into() } // These instructions can always be hoisted diff --git a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs index 51200c9b34e..252301433ad 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/remove_bit_shifts.rs @@ -171,7 +171,7 @@ impl Context<'_, '_, '_> { max_lhs_bits.checked_add(max_bit_shift_size).unwrap_or(FieldElement::max_num_bits()), FieldElement::max_num_bits(), ); - if max_bit <= typ.bit_size() { + if max_bit <= typ.bit_size::() { // If the result is guaranteed to fit in the target type we can simply multiply let pow = self.two_pow(rhs); let pow = self.insert_cast(pow, typ); @@ -183,13 +183,13 @@ impl Context<'_, '_, '_> { let lhs_field = self.insert_cast(lhs, NumericType::NativeField); // Unchecked mul as this is a wrapping operation that we later truncate let result = self.insert_binary(lhs_field, BinaryOp::Mul { unchecked: true }, pow); - let result = self.insert_truncate(result, typ.bit_size(), max_bit); + let result = self.insert_truncate(result, typ.bit_size::(), max_bit); self.insert_cast(result, typ) } else { // Otherwise, the result might not fit in a FieldElement. // For this, if we have to do `lhs << rhs` we can first shift by half of `rhs`, truncate, // then shift by `rhs - half_of_rhs` and truncate again. - assert!(typ.bit_size() <= 128); + assert!(typ.bit_size::() <= 128); let two = self.numeric_constant(FieldElement::from(2_u32), typ); @@ -209,9 +209,9 @@ impl Context<'_, '_, '_> { // = lhs * 2^(rhs_divided_by_two + rhs_remainder) = lhs * 2^rhs let lhs_field = self.insert_cast(lhs, NumericType::NativeField); let result = self.insert_binary(lhs_field, BinaryOp::Mul { unchecked: true }, pow1); - let result = self.insert_truncate(result, typ.bit_size(), max_bit); + let result = self.insert_truncate(result, typ.bit_size::(), max_bit); let result = self.insert_binary(result, BinaryOp::Mul { unchecked: true }, pow2); - let result = self.insert_truncate(result, typ.bit_size(), max_bit); + let result = self.insert_truncate(result, typ.bit_size::(), max_bit); self.insert_cast(result, typ) } } From 283007bfee8460e49bd8c0884f4f0a93e3580314 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 23 Oct 2025 08:52:49 -0300 Subject: [PATCH 2/5] Small adjustment --- compiler/noirc_evaluator/src/acir/types.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/noirc_evaluator/src/acir/types.rs b/compiler/noirc_evaluator/src/acir/types.rs index 2d1ee4547ad..41da2fd710e 100644 --- a/compiler/noirc_evaluator/src/acir/types.rs +++ b/compiler/noirc_evaluator/src/acir/types.rs @@ -7,7 +7,7 @@ use crate::{ ssa::ir::{types::NumericType, types::Type as SsaType}, }; use noirc_errors::call_stack::CallStack; -#[derive(Clone, Debug, PartialEq, Eq, Hash)] + /// High level Type descriptor for Variables. /// /// One can think of Expression/Witness/Const @@ -17,6 +17,7 @@ use noirc_errors::call_stack::CallStack; /// We could store this information when we do a range constraint /// but this information is readily available by the caller so /// we allow the user to pass it in. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub(crate) enum AcirType { NumericType(NumericType), Array(Vec, usize), From 342baeb1f886881d85a9bf26742901e7630c4d92 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 23 Oct 2025 08:54:43 -0300 Subject: [PATCH 3/5] Inline a few calls --- .../noirc_evaluator/src/acir/acir_context/mod.rs | 7 +++++-- compiler/noirc_evaluator/src/acir/types.rs | 12 ------------ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs index 6bf2936be8a..ed455ba48c3 100644 --- a/compiler/noirc_evaluator/src/acir/acir_context/mod.rs +++ b/compiler/noirc_evaluator/src/acir/acir_context/mod.rs @@ -281,7 +281,7 @@ impl AcirContext { predicate, BrilligStdlibFunc::Inverse, vec![AcirValue::Var(var, NumericType::NativeField)], - vec![AcirType::field()], + vec![AcirType::NumericType(NumericType::NativeField)], )?; Self::expect_one_var(results) }; @@ -866,7 +866,10 @@ impl AcirContext { AcirValue::Var(lhs, NumericType::unsigned(bit_size)), AcirValue::Var(rhs, NumericType::unsigned(bit_size)), ], - vec![AcirType::unsigned(max_q_bits), AcirType::unsigned(max_rhs_bits)], + vec![ + AcirType::NumericType(NumericType::unsigned(max_q_bits)), + AcirType::NumericType(NumericType::unsigned(max_rhs_bits)), + ], )? .try_into() .expect("quotient only returns two values"); diff --git a/compiler/noirc_evaluator/src/acir/types.rs b/compiler/noirc_evaluator/src/acir/types.rs index 41da2fd710e..68a1f27553d 100644 --- a/compiler/noirc_evaluator/src/acir/types.rs +++ b/compiler/noirc_evaluator/src/acir/types.rs @@ -23,18 +23,6 @@ pub(crate) enum AcirType { Array(Vec, usize), } -impl AcirType { - /// Returns a field type - pub(crate) fn field() -> Self { - AcirType::NumericType(NumericType::NativeField) - } - - /// Returns an unsigned type of the specified bit size - pub(crate) fn unsigned(bit_size: u32) -> Self { - AcirType::NumericType(NumericType::Unsigned { bit_size }) - } -} - impl From for AcirType { fn from(value: SsaType) -> Self { AcirType::from(&value) From 79d5d783692114ae59edd1e716fa859776a44263 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 23 Oct 2025 08:55:50 -0300 Subject: [PATCH 4/5] Remove unnecessary From impl --- compiler/noirc_evaluator/src/acir/types.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/compiler/noirc_evaluator/src/acir/types.rs b/compiler/noirc_evaluator/src/acir/types.rs index 68a1f27553d..d2844ba5c43 100644 --- a/compiler/noirc_evaluator/src/acir/types.rs +++ b/compiler/noirc_evaluator/src/acir/types.rs @@ -42,12 +42,6 @@ impl From<&SsaType> for AcirType { } } -impl From for AcirType { - fn from(value: NumericType) -> Self { - AcirType::NumericType(value) - } -} - #[derive(Clone)] pub(super) struct AcirDynamicArray { /// Identification for the Acir dynamic array From 9da3aada37f0208f04af3aede1a32c55d4354ce5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 23 Oct 2025 09:28:36 -0300 Subject: [PATCH 5/5] chore(ACIR): document AcirValue --- compiler/noirc_evaluator/src/acir/types.rs | 65 ++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/compiler/noirc_evaluator/src/acir/types.rs b/compiler/noirc_evaluator/src/acir/types.rs index d2844ba5c43..a50c5128bc7 100644 --- a/compiler/noirc_evaluator/src/acir/types.rs +++ b/compiler/noirc_evaluator/src/acir/types.rs @@ -42,6 +42,37 @@ impl From<&SsaType> for AcirType { } } +/// Represents an array whose size is not known during compile time, +/// and which might be indexed with a dynamic index. +/// +/// For example, in this Noir program: +/// +/// ```noir +/// fn main(array: [Field; 3], index: u32) -> pub Field { +/// array[index] +/// } +/// ``` +/// +/// the array, first represented as an [AcirValue::Array], will be put in +/// an [AcirValue::DynamicArray]: +/// +/// ```acir +/// private parameters: [w0, w1, w2, w3] +/// public parameters: [] +/// return values: [w4] +/// INIT b0 = [w0, w1, w2] +/// READ w5 = b0[w3] +/// ASSERT w4 = w5 +/// ``` +/// +/// Every dynamic array has an associated [BlockId] where its contents are stored, +/// in this case `b0`. Then the block can be read or written using dynamic indexes. +/// +/// Dynamic arrays might result from other operations. For example: +/// - setting the value of an array element with a dynamic index +/// - pushing back to a slice where it's length is not known at compile time +/// - inserting to a slice with a dynamic index +/// - removing from a slice at a dynamic index #[derive(Clone)] pub(super) struct AcirDynamicArray { /// Identification for the Acir dynamic array @@ -82,10 +113,44 @@ impl Debug for AcirDynamicArray { } } +/// All possible variants of values in ACIR. #[derive(Debug, Clone)] pub(crate) enum AcirValue { + /// Represents a single numeric value in ACIR. Var(AcirVar, NumericType), + /// Represents a fixed-size array of ACIR values that are never indexed with + /// a dynamic index. + /// + /// For example, in this Noir program: + /// + /// ```noir + /// fn main(array: [Field; 3]) { + /// // Safety: example + /// unsafe { call(array) }; + /// } + /// + /// unconstrained fn call(array: [Field; 3]) { + /// println(array); + /// } + /// ``` + /// + /// the array will be represented as an [AcirValue::Array] + /// but we can see that it's simply a list of variables: + /// + /// ```acir + /// private parameters: [w0, w1, w2] + /// public parameters: [] + /// return values: [] + /// BRILLIG CALL func: 0, inputs: [[w0, w1, w2]], outputs: [] + /// ``` + /// + /// Compare this with `DynamicArray` below. Array(im::Vector), + /// Represents an array whose size is not known during compile time, + /// and which might be indexed with a dynamic index. + /// + /// See [AcirDynamicArray] for more details and how this differs from + /// [AcirValue::Array]. DynamicArray(AcirDynamicArray), }