Skip to content

Commit

Permalink
chore(ssa): indent NumericType into ObjectType (#810)
Browse files Browse the repository at this point in the history
* - indent NumericType into ObjectType
- Create helper methods for common operations involving NumericType

* fix import - submodules were using `node` path from the root

* boolean is now seen as `u1`

* add comment re strings

* Merge remote-tracking branch 'origin/master' into kw/dedup-numeric-type

---------

Co-authored-by: Joss <[email protected]>
  • Loading branch information
kevaundray and joss-aztec authored Mar 31, 2023
1 parent ce61349 commit a901967
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 107 deletions.
6 changes: 3 additions & 3 deletions crates/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use errors::{RuntimeError, RuntimeErrorKind};
use iter_extended::btree_map;
use noirc_abi::{Abi, AbiType, AbiVisibility, MAIN_RETURN_NAME};
use noirc_frontend::monomorphization::ast::*;
use ssa::{node, ssa_gen::IrGenerator};
use ssa::{node::ObjectType, ssa_gen::IrGenerator};
use std::collections::{BTreeMap, BTreeSet};

#[derive(Default)]
Expand Down Expand Up @@ -165,7 +165,7 @@ impl Evaluator {
ir_gen.create_new_variable(
name.to_owned(),
Some(def),
node::ObjectType::NativeField,
ObjectType::native_field(),
Some(witness),
);
vec![witness]
Expand All @@ -187,7 +187,7 @@ impl Evaluator {
AbiType::Boolean => {
let witness = self.add_witness_to_cs();
ssa::acir_gen::range_constraint(witness, 1, self)?;
let obj_type = node::ObjectType::Boolean;
let obj_type = ObjectType::boolean();
ir_gen.create_new_variable(name.to_owned(), Some(def), obj_type, Some(witness));

vec![witness]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,7 @@ impl InternalVarCache {
NodeObject::Variable(variable) => {
let variable_type = variable.get_type();
match variable_type {
ObjectType::Boolean
| ObjectType::NativeField
| ObjectType::Signed(..)
| ObjectType::Unsigned(..) => {
ObjectType::Numeric(..) => {
let witness =
variable.witness.unwrap_or_else(|| evaluator.add_witness_to_cs());
InternalVar::from_witness(witness)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub(crate) fn evaluate(
BinaryOp::Sub { max_rhs_value } | BinaryOp::SafeSub { max_rhs_value } => {
let l_c = acir_gen.var_cache.get_or_compute_internal_var_unwrap(binary.lhs, evaluator, ctx);
let r_c = acir_gen.var_cache.get_or_compute_internal_var_unwrap(binary.rhs, evaluator, ctx);
if res_type == ObjectType::NativeField {
if res_type == ObjectType::native_field() {
InternalVar::from(constraints::subtract(
l_c.expression(),
FieldElement::one(),
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/anchor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(super) enum CseAction {
#[derive(Default, Clone)]
pub(super) struct Anchor {
map: HashMap<Opcode, HashMap<Operation, NodeId>>, //standard anchor
cast_map: HashMap<NodeId, HashMap<crate::node::ObjectType, NodeId>>, //cast anchor
cast_map: HashMap<NodeId, HashMap<ObjectType, NodeId>>, //cast anchor
mem_map: HashMap<ArrayId, Vec<VecDeque<(usize, NodeId)>>>, //Memory anchor: one Vec for each array where Vec[i] contains the list of load and store instructions having index i, and the mem_item position in which they appear
mem_list: HashMap<ArrayId, VecDeque<MemItem>>, // list of the memory instructions, per array, and grouped into MemItems
}
Expand Down
20 changes: 11 additions & 9 deletions crates/noirc_evaluator/src/ssa/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl Opcode {
BlackBoxFunc::SchnorrVerify
| BlackBoxFunc::EcdsaSecp256k1
| BlackBoxFunc::MerkleMembership => BigUint::one(),
BlackBoxFunc::HashToField128Security => ObjectType::NativeField.max_size(),
BlackBoxFunc::HashToField128Security => ObjectType::native_field().max_size(),
BlackBoxFunc::AES => {
todo!("ICE: AES is unimplemented")
}
Expand Down Expand Up @@ -108,21 +108,23 @@ impl Opcode {
BlackBoxFunc::Keccak256 => {
todo!("ICE: Keccak256 is unimplemented")
}
BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => (32, ObjectType::Unsigned(8)),
BlackBoxFunc::HashToField128Security => (1, ObjectType::NativeField),
BlackBoxFunc::SHA256 | BlackBoxFunc::Blake2s => {
(32, ObjectType::unsigned_integer(8))
}
BlackBoxFunc::HashToField128Security => (1, ObjectType::native_field()),
// See issue #775 on changing this to return a boolean
BlackBoxFunc::MerkleMembership
| BlackBoxFunc::SchnorrVerify
| BlackBoxFunc::EcdsaSecp256k1 => (1, ObjectType::NativeField),
BlackBoxFunc::Pedersen => (2, ObjectType::NativeField),
BlackBoxFunc::FixedBaseScalarMul => (2, ObjectType::NativeField),
| BlackBoxFunc::EcdsaSecp256k1 => (1, ObjectType::native_field()),
BlackBoxFunc::Pedersen => (2, ObjectType::native_field()),
BlackBoxFunc::FixedBaseScalarMul => (2, ObjectType::native_field()),
BlackBoxFunc::RANGE | BlackBoxFunc::AND | BlackBoxFunc::XOR => {
unreachable!("ICE: these opcodes do not have Noir builtin functions")
}
}
}
Opcode::ToBits(_) => (FieldElement::max_num_bits(), ObjectType::Boolean),
Opcode::ToRadix(_) => (FieldElement::max_num_bits(), ObjectType::NativeField),
Opcode::ToBits(_) => (FieldElement::max_num_bits(), ObjectType::boolean()),
Opcode::ToRadix(_) => (FieldElement::max_num_bits(), ObjectType::native_field()),
Opcode::Println(_) => (0, ObjectType::NotAnObject),
Opcode::Sort => {
let a = super::mem::Memory::deref(ctx, args[0]).unwrap();
Expand All @@ -135,7 +137,7 @@ impl Opcode {
#[derive(Clone, Debug, Hash, Copy, PartialEq, Eq)]
pub(crate) struct PrintlnInfo {
// We store strings as arrays and there is no differentiation between them in the SSA.
// This bool simply states whether an array that is to be printed should be outputted as a utf8 string
// This bool simply states whether an array that is to be printed should be outputted as a utf8 string.
pub(crate) is_string_output: bool,
// This is a flag used during `nargo test` to determine whether to display println output.
pub(crate) show_output: bool,
Expand Down
20 changes: 10 additions & 10 deletions crates/noirc_evaluator/src/ssa/conditional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
context::SsaContext,
flatten::UnrollContext,
inline::StackFrame,
node::{BinaryOp, Instruction, Mark, NodeId, ObjectType, Opcode, Operation},
node::{Binary, BinaryOp, Instruction, Mark, NodeId, ObjectType, Opcode, Operation},
{block, flatten, node, optimizations},
},
};
Expand Down Expand Up @@ -171,7 +171,7 @@ impl DecisionTree {
BinaryOp::Mul,
parent_value,
condition,
ObjectType::Boolean,
ObjectType::boolean(),
)
} else {
let not_condition = DecisionTree::new_instruction_after_phi(
Expand All @@ -180,15 +180,15 @@ impl DecisionTree {
BinaryOp::Sub { max_rhs_value: BigUint::one() },
ctx.one(),
condition,
ObjectType::Boolean,
ObjectType::boolean(),
);
DecisionTree::new_instruction_after(
ctx,
block_id,
BinaryOp::Mul,
parent_value,
not_condition,
ObjectType::Boolean,
ObjectType::boolean(),
not_condition,
)
};
Expand Down Expand Up @@ -480,7 +480,7 @@ impl DecisionTree {
Operation::Cond { condition, val_true: ctx.zero(), val_false: ctx.one() };
let cond = ctx.add_instruction(Instruction::new(
operation,
ObjectType::Boolean,
ObjectType::boolean(),
Some(stack.block),
));
stack.push(cond);
Expand Down Expand Up @@ -595,7 +595,7 @@ impl DecisionTree {
});
cond = ctx.add_instruction(Instruction::new(
op,
ObjectType::Boolean,
ObjectType::boolean(),
Some(stack.block),
));
optimizations::simplify_id(ctx, cond).unwrap();
Expand Down Expand Up @@ -624,7 +624,7 @@ impl DecisionTree {
}
if ctx.under_assumption(cond) {
let ins2 = ctx.instruction_mut(ins_id);
ins2.operation = Operation::Binary(crate::node::Binary {
ins2.operation = Operation::Binary(Binary {
lhs: binary_op.lhs,
rhs: binary_op.rhs,
operator: binary_op.operator.clone(),
Expand Down Expand Up @@ -724,7 +724,7 @@ impl DecisionTree {
}
let cond = ctx.add_instruction(Instruction::new(
operation,
ObjectType::Boolean,
ObjectType::boolean(),
Some(stack.block),
));
stack.push(cond);
Expand Down Expand Up @@ -775,7 +775,7 @@ impl DecisionTree {
});
let cond = ctx.add_instruction(Instruction::new(
op,
ObjectType::Boolean,
ObjectType::boolean(),
Some(stack_frame.block),
));
optimizations::simplify_id(ctx, cond).unwrap();
Expand Down Expand Up @@ -808,7 +808,7 @@ impl DecisionTree {
});
let cond = ctx.add_instruction(Instruction::new(
op,
ObjectType::Boolean,
ObjectType::boolean(),
Some(stack_frame.block),
));
optimizations::simplify_id(ctx, cond).unwrap();
Expand Down
60 changes: 35 additions & 25 deletions crates/noirc_evaluator/src/ssa/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,19 @@ impl Default for SsaContext {
constants: HashMap::new(),
};
block::create_first_block(&mut pc);
pc.one_with_type(node::ObjectType::Boolean);
pc.zero_with_type(node::ObjectType::Boolean);
pc.one_with_type(ObjectType::boolean());
pc.zero_with_type(ObjectType::boolean());
pc
}
}

impl SsaContext {
pub(crate) fn zero(&self) -> NodeId {
self.find_const_with_type(&FieldElement::zero(), node::ObjectType::Boolean).unwrap()
self.find_const_with_type(&FieldElement::zero(), ObjectType::boolean()).unwrap()
}

pub(crate) fn one(&self) -> NodeId {
self.find_const_with_type(&FieldElement::one(), node::ObjectType::Boolean).unwrap()
self.find_const_with_type(&FieldElement::one(), ObjectType::boolean()).unwrap()
}

pub(crate) fn zero_with_type(&mut self, obj_type: ObjectType) -> NodeId {
Expand Down Expand Up @@ -139,7 +139,7 @@ impl SsaContext {
predicate: None,
location: None,
};
let dummy_store = node::Instruction::new(op_a, node::ObjectType::NotAnObject, None);
let dummy_store = node::Instruction::new(op_a, ObjectType::NotAnObject, None);
let id = self.add_instruction(dummy_store);
self.dummy_store.insert(a, id);
}
Expand Down Expand Up @@ -621,7 +621,7 @@ impl SsaContext {
| Binary(node::Binary { operator: Slt, .. })
| Binary(node::Binary { operator: Sle, .. })
| Binary(node::Binary { operator: Lt, .. })
| Binary(node::Binary { operator: Lte, .. }) => ObjectType::Boolean,
| Binary(node::Binary { operator: Lte, .. }) => ObjectType::boolean(),
Operation::Jne(_, _)
| Operation::Jeq(_, _)
| Operation::Jmp(_)
Expand Down Expand Up @@ -649,7 +649,7 @@ impl SsaContext {
//we create a variable pointing to this MemArray
let new_var = node::Variable {
id: NodeId::dummy(),
obj_type: node::ObjectType::Pointer(array_index),
obj_type: ObjectType::Pointer(array_index),
name: name.to_string(),
root: None,
def: def.clone(),
Expand Down Expand Up @@ -786,10 +786,14 @@ impl SsaContext {
let len = self.mem[a].len;
let e_type = self.mem[b].element_type;
for i in 0..len {
let idx_b = self
.get_or_create_const(FieldElement::from(i as i128), ObjectType::Unsigned(32));
let idx_a = self
.get_or_create_const(FieldElement::from(i as i128), ObjectType::Unsigned(32));
let idx_b = self.get_or_create_const(
FieldElement::from(i as i128),
ObjectType::unsigned_integer(32),
);
let idx_a = self.get_or_create_const(
FieldElement::from(i as i128),
ObjectType::unsigned_integer(32),
);
let op_b = Operation::Load { array_id: b, index: idx_b, location: None };
let load = self.new_instruction(op_b, e_type)?;
let op_a = Operation::Store {
Expand Down Expand Up @@ -957,8 +961,10 @@ impl SsaContext {
let e_type = self.mem[array_id].element_type;
assert_eq!(len, values.len());
for (i, v) in values.iter().enumerate() {
let index =
self.get_or_create_const(FieldElement::from(i as i128), ObjectType::Unsigned(32));
let index = self.get_or_create_const(
FieldElement::from(i as i128),
ObjectType::unsigned_integer(32),
);
let op_a =
Operation::Store { array_id, index, value: *v, predicate: None, location: None };
self.new_instruction_inline(op_a, e_type, stack_frame);
Expand All @@ -979,10 +985,14 @@ impl SsaContext {
let len = self.mem[a].len;
let e_type = self.mem[b].element_type;
for i in 0..len {
let idx_b = self
.get_or_create_const(FieldElement::from(i as i128), ObjectType::Unsigned(32));
let idx_a = self
.get_or_create_const(FieldElement::from(i as i128), ObjectType::Unsigned(32));
let idx_b = self.get_or_create_const(
FieldElement::from(i as i128),
ObjectType::unsigned_integer(32),
);
let idx_a = self.get_or_create_const(
FieldElement::from(i as i128),
ObjectType::unsigned_integer(32),
);
let op_b = Operation::Load { array_id: b, index: idx_b, location: None };
let load = self.new_instruction_inline(op_b, e_type, stack_frame);
let op_a = Operation::Store {
Expand Down Expand Up @@ -1071,13 +1081,13 @@ impl SsaContext {

let name = format!("if_{}_ret{c}", exit_block.0.into_raw_parts().0);
*c += 1;
if let node::ObjectType::Pointer(adr1) = a_type {
if let ObjectType::Pointer(adr1) = a_type {
let len = self.mem[adr1].len;
let el_type = self.mem[adr1].element_type;
let (id, array_id) = self.new_array(&name, el_type, len, None);
for i in 0..len {
let index = self
.get_or_create_const(FieldElement::from(i as u128), ObjectType::NativeField);
.get_or_create_const(FieldElement::from(i as u128), ObjectType::native_field());
self.current_block = block1;
let op = Operation::Load { array_id: adr1, index, location: None };
let v1 = self.new_instruction(op, el_type).unwrap();
Expand Down Expand Up @@ -1178,16 +1188,16 @@ impl SsaContext {
pub(crate) fn convert_type(&mut self, t: &Type) -> ObjectType {
use noirc_frontend::Signedness;
match t {
Type::Bool => ObjectType::Boolean,
Type::Field => ObjectType::NativeField,
Type::Bool => ObjectType::boolean(),
Type::Field => ObjectType::native_field(),
Type::Integer(sign, bit_size) => {
assert!(
*bit_size < super::integer::short_integer_max_bit_size(),
"long integers are not yet supported"
);
match sign {
Signedness::Signed => ObjectType::Signed(*bit_size),
Signedness::Unsigned => ObjectType::Unsigned(*bit_size),
Signedness::Signed => ObjectType::signed_integer(*bit_size),
Signedness::Unsigned => ObjectType::unsigned_integer(*bit_size),
}
}
Type::Array(..) => panic!("Cannot convert an array type {t} into an ObjectType since it is unknown which array it refers to"),
Expand Down Expand Up @@ -1222,7 +1232,7 @@ impl SsaContext {
});
let cond = self.add_instruction(Instruction::new(
op,
ObjectType::Boolean,
ObjectType::boolean(),
Some(stack.block),
));
optimizations::simplify_id(self, cond).unwrap();
Expand All @@ -1239,7 +1249,7 @@ impl SsaContext {
Operation::Cond { condition: pred, val_true: *cond, val_false: self.one() };
let c_ins = self.add_instruction(Instruction::new(
operation,
ObjectType::Boolean,
ObjectType::boolean(),
Some(stack.block),
));
stack.push(c_ins);
Expand Down
13 changes: 7 additions & 6 deletions crates/noirc_evaluator/src/ssa/integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ fn get_instruction_max_operand(
Operation::Binary(node::Binary { operator, lhs, rhs, .. }) => {
if let BinaryOp::Sub { .. } = operator {
//TODO uses interval analysis instead
if matches!(ins.res_type, ObjectType::Unsigned(_) | ObjectType::Boolean) {
// Note that a boolean is also handled as an unsigned integer
if ins.res_type.is_unsigned_integer() {
if let Some(lhs_const) = ctx.get_as_constant(*lhs) {
let lhs_big = BigUint::from_bytes_be(&lhs_const.to_be_bytes());
if max_map[rhs] <= lhs_big {
Expand Down Expand Up @@ -266,14 +267,14 @@ fn block_overflow(
let ins_max_bits = get_instruction_max(ctx, &ins, max_map, &value_map).bits();
let res_type = ins.res_type;

let too_many_bits = ins_max_bits > FieldElement::max_num_bits() as u64
&& res_type != ObjectType::NativeField;
let too_many_bits =
ins_max_bits > FieldElement::max_num_bits() as u64 && !res_type.is_native_field();

ins.operation.for_each_id(|id| {
get_obj_max_value(ctx, id, max_map, &value_map);
let arg = ctx.try_get_node(id);
let should_truncate_arg =
should_truncate_ins && arg.is_some() && get_type(arg) != ObjectType::NativeField;
should_truncate_ins && arg.is_some() && !get_type(arg).is_native_field();

if should_truncate_arg || too_many_bits {
add_to_truncate(ctx, id, get_size_in_bits(arg), &mut truncate_map, max_map);
Expand Down Expand Up @@ -312,7 +313,7 @@ fn block_overflow(
}
}
Operation::Binary(node::Binary { operator: BinaryOp::Shr(loc), lhs, rhs, .. }) => {
if !matches!(ins.res_type, node::ObjectType::Unsigned(_)) {
if !ins.res_type.is_unsigned_integer() {
todo!("Right shift is only implemented for unsigned integers");
}
if let Some(r_const) = ctx.get_as_constant(rhs) {
Expand Down Expand Up @@ -494,7 +495,7 @@ fn get_max_value(ins: &Instruction, max_map: &mut HashMap<NodeId, BigUint>) -> B
Operation::Intrinsic(opcode, _) => opcode.get_max_value(),
};

if ins.res_type == ObjectType::NativeField {
if ins.res_type.is_native_field() {
let field_max = BigUint::from_bytes_be(&FieldElement::one().neg().to_be_bytes());

//Native Field operations cannot overflow so they will not be truncated
Expand Down
Loading

0 comments on commit a901967

Please sign in to comment.