Skip to content

Commit

Permalink
chore: Use NumericType not Type for casts and numeric constants (#…
Browse files Browse the repository at this point in the history
…6769)

Co-authored-by: Tom French <[email protected]>
  • Loading branch information
jfecher and TomAFrench authored Dec 11, 2024
1 parent 2a840ca commit e973397
Show file tree
Hide file tree
Showing 30 changed files with 376 additions and 355 deletions.
18 changes: 12 additions & 6 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1872,7 +1872,8 @@ impl<'a> Context<'a> {

let acir_value = match value {
Value::NumericConstant { constant, typ } => {
AcirValue::Var(self.acir_context.add_constant(*constant), typ.into())
let typ = AcirType::from(Type::Numeric(*typ));
AcirValue::Var(self.acir_context.add_constant(*constant), typ)
}
Value::Intrinsic(..) => todo!(),
Value::Function(function_id) => {
Expand Down Expand Up @@ -2902,7 +2903,12 @@ mod test {
brillig::Brillig,
ssa::{
function_builder::FunctionBuilder,
ir::{function::FunctionId, instruction::BinaryOp, map::Id, types::Type},
ir::{
function::FunctionId,
instruction::BinaryOp,
map::Id,
types::{NumericType, Type},
},
},
};

Expand Down Expand Up @@ -2930,7 +2936,7 @@ mod test {
let foo_v1 = builder.add_parameter(Type::field());

let foo_equality_check = builder.insert_binary(foo_v0, BinaryOp::Eq, foo_v1);
let zero = builder.numeric_constant(0u128, Type::unsigned(1));
let zero = builder.numeric_constant(0u128, NumericType::unsigned(1));
builder.insert_constrain(foo_equality_check, zero, None);
builder.terminate_with_return(vec![foo_v0]);
}
Expand Down Expand Up @@ -3374,7 +3380,7 @@ mod test {

// Call the same primitive operation again
let v1_div_v2 = builder.insert_binary(main_v1, BinaryOp::Div, main_v2);
let one = builder.numeric_constant(1u128, Type::unsigned(32));
let one = builder.numeric_constant(1u128, NumericType::unsigned(32));
builder.insert_constrain(v1_div_v2, one, None);

builder.terminate_with_return(vec![]);
Expand Down Expand Up @@ -3447,7 +3453,7 @@ mod test {

// Call the same primitive operation again
let v1_div_v2 = builder.insert_binary(main_v1, BinaryOp::Div, main_v2);
let one = builder.numeric_constant(1u128, Type::unsigned(32));
let one = builder.numeric_constant(1u128, NumericType::unsigned(32));
builder.insert_constrain(v1_div_v2, one, None);

builder.terminate_with_return(vec![]);
Expand Down Expand Up @@ -3533,7 +3539,7 @@ mod test {

// Call the same primitive operation again
let v1_div_v2 = builder.insert_binary(main_v1, BinaryOp::Div, main_v2);
let one = builder.numeric_constant(1u128, Type::unsigned(32));
let one = builder.numeric_constant(1u128, NumericType::unsigned(32));
builder.insert_constrain(v1_div_v2, one, None);

builder.terminate_with_return(vec![]);
Expand Down
22 changes: 10 additions & 12 deletions compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,16 +226,14 @@ impl<'block> BrilligBlock<'block> {
dfg.get_numeric_constant_with_type(*rhs),
) {
// If the constraint is of the form `x == u1 1` then we can simply constrain `x` directly
(
Some((constant, Type::Numeric(NumericType::Unsigned { bit_size: 1 }))),
None,
) if constant == FieldElement::one() => {
(Some((constant, NumericType::Unsigned { bit_size: 1 })), None)
if constant == FieldElement::one() =>
{
(self.convert_ssa_single_addr_value(*rhs, dfg), false)
}
(
None,
Some((constant, Type::Numeric(NumericType::Unsigned { bit_size: 1 }))),
) if constant == FieldElement::one() => {
(None, Some((constant, NumericType::Unsigned { bit_size: 1 })))
if constant == FieldElement::one() =>
{
(self.convert_ssa_single_addr_value(*lhs, dfg), false)
}

Expand Down Expand Up @@ -1285,8 +1283,8 @@ impl<'block> BrilligBlock<'block> {
result_variable: SingleAddrVariable,
) {
let binary_type = type_of_binary_operation(
dfg[binary.lhs].get_type(),
dfg[binary.rhs].get_type(),
dfg[binary.lhs].get_type().as_ref(),
dfg[binary.rhs].get_type().as_ref(),
binary.operator,
);

Expand Down Expand Up @@ -1795,7 +1793,7 @@ impl<'block> BrilligBlock<'block> {
dfg: &DataFlowGraph,
) -> BrilligVariable {
let typ = dfg[result].get_type();
match typ {
match typ.as_ref() {
Type::Numeric(_) => self.variables.define_variable(
self.function_context,
self.brillig_context,
Expand All @@ -1811,7 +1809,7 @@ impl<'block> BrilligBlock<'block> {
dfg,
);
let array = variable.extract_array();
self.allocate_foreign_call_result_array(typ, array);
self.allocate_foreign_call_result_array(typ.as_ref(), array);

variable
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ mod test {

let v3 = builder.insert_allocate(Type::field());

let zero = builder.numeric_constant(0u128, Type::field());
let zero = builder.field_constant(0u128);
builder.insert_store(v3, zero);

let v4 = builder.insert_binary(v0, BinaryOp::Eq, zero);
Expand All @@ -381,7 +381,7 @@ mod test {

builder.switch_to_block(b2);

let twenty_seven = builder.numeric_constant(27u128, Type::field());
let twenty_seven = builder.field_constant(27u128);
let v7 = builder.insert_binary(v0, BinaryOp::Add, twenty_seven);
builder.insert_store(v3, v7);

Expand Down Expand Up @@ -487,7 +487,7 @@ mod test {

let v3 = builder.insert_allocate(Type::field());

let zero = builder.numeric_constant(0u128, Type::field());
let zero = builder.field_constant(0u128);
builder.insert_store(v3, zero);

builder.terminate_with_jmp(b1, vec![zero]);
Expand Down Expand Up @@ -515,7 +515,7 @@ mod test {

builder.switch_to_block(b5);

let twenty_seven = builder.numeric_constant(27u128, Type::field());
let twenty_seven = builder.field_constant(27u128);
let v10 = builder.insert_binary(v7, BinaryOp::Eq, twenty_seven);

let v11 = builder.insert_not(v10);
Expand All @@ -534,7 +534,7 @@ mod test {

builder.switch_to_block(b8);

let one = builder.numeric_constant(1u128, Type::field());
let one = builder.field_constant(1u128);
let v15 = builder.insert_binary(v7, BinaryOp::Add, one);

builder.terminate_with_jmp(b4, vec![v15]);
Expand Down
16 changes: 10 additions & 6 deletions compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
use std::{collections::BTreeMap, sync::Arc};

use crate::ssa::ir::{function::RuntimeType, types::Type, value::ValueId};
use crate::ssa::ir::{
function::RuntimeType,
types::{NumericType, Type},
value::ValueId,
};
use acvm::FieldElement;
use fxhash::FxHashMap as HashMap;
use noirc_frontend::ast;
Expand Down Expand Up @@ -115,7 +119,7 @@ impl FunctionBuilder {
/// Insert a value into a data bus builder
fn add_to_data_bus(&mut self, value: ValueId, databus: &mut DataBusBuilder) {
assert!(databus.databus.is_none(), "initializing finalized call data");
let typ = self.current_function.dfg[value].get_type().clone();
let typ = self.current_function.dfg[value].get_type().into_owned();
match typ {
Type::Numeric(_) => {
databus.values.push_back(value);
Expand All @@ -128,10 +132,10 @@ impl FunctionBuilder {
for _i in 0..len {
for subitem_typ in typ.iter() {
// load each element of the array, and add it to the databus
let index_var = self
.current_function
.dfg
.make_constant(FieldElement::from(index as i128), Type::length_type());
let length_type = NumericType::length_type();
let index_var = FieldElement::from(index as i128);
let index_var =
self.current_function.dfg.make_constant(index_var, length_type);
let element = self.insert_array_get(value, index_var, subitem_typ.clone());
index += match subitem_typ {
Type::Array(_, _) | Type::Slice(_) => subitem_typ.element_size(),
Expand Down
19 changes: 10 additions & 9 deletions compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use super::{
dfg::{CallStack, InsertInstructionResult},
function::RuntimeType,
instruction::{ConstrainError, InstructionId, Intrinsic},
types::NumericType,
},
ssa_gen::Ssa,
};
Expand Down Expand Up @@ -122,19 +123,19 @@ impl FunctionBuilder {
pub(crate) fn numeric_constant(
&mut self,
value: impl Into<FieldElement>,
typ: Type,
typ: NumericType,
) -> ValueId {
self.current_function.dfg.make_constant(value.into(), typ)
}

/// Insert a numeric constant into the current function of type Field
pub(crate) fn field_constant(&mut self, value: impl Into<FieldElement>) -> ValueId {
self.numeric_constant(value.into(), Type::field())
self.numeric_constant(value.into(), NumericType::NativeField)
}

/// Insert a numeric constant into the current function of type Type::length_type()
pub(crate) fn length_constant(&mut self, value: impl Into<FieldElement>) -> ValueId {
self.numeric_constant(value.into(), Type::length_type())
self.numeric_constant(value.into(), NumericType::length_type())
}

/// Returns the type of the given value.
Expand Down Expand Up @@ -251,7 +252,7 @@ impl FunctionBuilder {

/// 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 {
pub(crate) fn insert_cast(&mut self, value: ValueId, typ: NumericType) -> ValueId {
self.insert_instruction(Instruction::Cast(value, typ), None).first()
}

Expand Down Expand Up @@ -526,7 +527,7 @@ mod tests {
use crate::ssa::ir::{
instruction::{Endian, Intrinsic},
map::Id,
types::Type,
types::{NumericType, Type},
};

use super::FunctionBuilder;
Expand All @@ -538,12 +539,12 @@ mod tests {
// let bits: [u1; 8] = x.to_le_bits();
let func_id = Id::test_new(0);
let mut builder = FunctionBuilder::new("func".into(), func_id);
let one = builder.numeric_constant(FieldElement::one(), Type::bool());
let zero = builder.numeric_constant(FieldElement::zero(), Type::bool());
let one = builder.numeric_constant(FieldElement::one(), NumericType::bool());
let zero = builder.numeric_constant(FieldElement::zero(), NumericType::bool());

let to_bits_id = builder.import_intrinsic_id(Intrinsic::ToBits(Endian::Little));
let input = builder.numeric_constant(FieldElement::from(7_u128), Type::field());
let length = builder.numeric_constant(FieldElement::from(8_u128), Type::field());
let input = builder.field_constant(FieldElement::from(7_u128));
let length = builder.field_constant(FieldElement::from(8_u128));
let result_types = vec![Type::Array(Arc::new(vec![Type::bool()]), 8)];
let call_results =
builder.insert_call(to_bits_id, vec![input, length], result_types).into_owned();
Expand Down
27 changes: 14 additions & 13 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use super::{
Instruction, InstructionId, InstructionResultType, Intrinsic, TerminatorInstruction,
},
map::DenseMap,
types::Type,
types::{NumericType, Type},
value::{Value, ValueId},
};

Expand Down Expand Up @@ -50,7 +50,7 @@ pub(crate) struct DataFlowGraph {
/// Each constant is unique, attempting to insert the same constant
/// twice will return the same ValueId.
#[serde(skip)]
constants: HashMap<(FieldElement, Type), ValueId>,
constants: HashMap<(FieldElement, NumericType), ValueId>,

/// Contains each function that has been imported into the current function.
/// A unique `ValueId` for each function's [`Value::Function`] is stored so any given FunctionId
Expand Down Expand Up @@ -119,7 +119,7 @@ impl DataFlowGraph {
let parameters = self.blocks[block].parameters();

let parameters = vecmap(parameters.iter().enumerate(), |(position, param)| {
let typ = self.values[*param].get_type().clone();
let typ = self.values[*param].get_type().into_owned();
self.values.insert(Value::Param { block: new_block, position, typ })
});

Expand Down Expand Up @@ -233,11 +233,12 @@ impl DataFlowGraph {
pub(crate) fn set_type_of_value(&mut self, value_id: ValueId, target_type: Type) {
let value = &mut self.values[value_id];
match value {
Value::Instruction { typ, .. }
| Value::Param { typ, .. }
| Value::NumericConstant { typ, .. } => {
Value::Instruction { typ, .. } | Value::Param { typ, .. } => {
*typ = target_type;
}
Value::NumericConstant { typ, .. } => {
*typ = target_type.unwrap_numeric();
}
_ => {
unreachable!("ICE: Cannot set type of {:?}", value);
}
Expand All @@ -257,11 +258,11 @@ impl DataFlowGraph {

/// Creates a new constant value, or returns the Id to an existing one if
/// one already exists.
pub(crate) fn make_constant(&mut self, constant: FieldElement, typ: Type) -> ValueId {
if let Some(id) = self.constants.get(&(constant, typ.clone())) {
pub(crate) fn make_constant(&mut self, constant: FieldElement, typ: NumericType) -> ValueId {
if let Some(id) = self.constants.get(&(constant, typ)) {
return *id;
}
let id = self.values.insert(Value::NumericConstant { constant, typ: typ.clone() });
let id = self.values.insert(Value::NumericConstant { constant, typ });
self.constants.insert((constant, typ), id);
id
}
Expand Down Expand Up @@ -342,7 +343,7 @@ impl DataFlowGraph {

/// Returns the type of a given value
pub(crate) fn type_of_value(&self, value: ValueId) -> Type {
self.values[value].get_type().clone()
self.values[value].get_type().into_owned()
}

/// Returns the maximum possible number of bits that `value` can potentially be.
Expand All @@ -367,7 +368,7 @@ impl DataFlowGraph {
/// True if the type of this value is Type::Reference.
/// Using this method over type_of_value avoids cloning the value's type.
pub(crate) fn value_is_reference(&self, value: ValueId) -> bool {
matches!(self.values[value].get_type(), Type::Reference(_))
matches!(self.values[value].get_type().as_ref(), Type::Reference(_))
}

/// Replaces an instruction result with a fresh id.
Expand Down Expand Up @@ -425,9 +426,9 @@ impl DataFlowGraph {
pub(crate) fn get_numeric_constant_with_type(
&self,
value: ValueId,
) -> Option<(FieldElement, Type)> {
) -> Option<(FieldElement, NumericType)> {
match &self.values[self.resolve(value)] {
Value::NumericConstant { constant, typ } => Some((*constant, typ.clone())),
Value::NumericConstant { constant, typ } => Some((*constant, *typ)),
_ => None,
}
}
Expand Down
15 changes: 7 additions & 8 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ pub(crate) enum Instruction {
/// Binary Operations like +, -, *, /, ==, !=
Binary(Binary),

/// Converts `Value` into Typ
Cast(ValueId, Type),
/// Converts `Value` into the given NumericType
Cast(ValueId, NumericType),

/// Computes a bit wise not
Not(ValueId),
Expand Down Expand Up @@ -355,9 +355,8 @@ impl Instruction {
pub(crate) fn result_type(&self) -> InstructionResultType {
match self {
Instruction::Binary(binary) => binary.result_type(),
Instruction::Cast(_, typ) | Instruction::MakeArray { typ, .. } => {
InstructionResultType::Known(typ.clone())
}
Instruction::Cast(_, typ) => InstructionResultType::Known(Type::Numeric(*typ)),
Instruction::MakeArray { typ, .. } => InstructionResultType::Known(typ.clone()),
Instruction::Not(value)
| Instruction::Truncate { value, .. }
| Instruction::ArraySet { array: value, .. }
Expand Down Expand Up @@ -603,7 +602,7 @@ impl Instruction {
rhs: f(binary.rhs),
operator: binary.operator,
}),
Instruction::Cast(value, typ) => Instruction::Cast(f(*value), typ.clone()),
Instruction::Cast(value, typ) => Instruction::Cast(f(*value), *typ),
Instruction::Not(value) => Instruction::Not(f(*value)),
Instruction::Truncate { value, bit_size, max_bit_size } => Instruction::Truncate {
value: f(*value),
Expand Down Expand Up @@ -751,7 +750,7 @@ impl Instruction {
use SimplifyResult::*;
match self {
Instruction::Binary(binary) => binary.simplify(dfg),
Instruction::Cast(value, typ) => simplify_cast(*value, typ, dfg),
Instruction::Cast(value, typ) => simplify_cast(*value, *typ, dfg),
Instruction::Not(value) => {
match &dfg[dfg.resolve(*value)] {
// Limit optimizing ! on constants to only booleans. If we tried it on fields,
Expand All @@ -760,7 +759,7 @@ impl Instruction {
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 value = !constant.to_u128() % (1 << typ.bit_size());
SimplifiedTo(dfg.make_constant(value.into(), typ.clone()))
SimplifiedTo(dfg.make_constant(value.into(), *typ))
}
Value::Instruction { instruction, .. } => {
// !!v => v
Expand Down
Loading

0 comments on commit e973397

Please sign in to comment.