Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Use NumericType not Type for casts and numeric constants #6769

Merged
merged 3 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
/// 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 @@
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 @@ -407,7 +406,7 @@
// These can fail.
Constrain(..) | RangeCheck { .. } => true,

// This should never be side-effectful

Check warning on line 409 in compiler/noirc_evaluator/src/ssa/ir/instruction.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (effectful)
MakeArray { .. } => false,

// Some binary math can overflow or underflow
Expand Down Expand Up @@ -603,7 +602,7 @@
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 @@
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 @@
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
Loading