Skip to content

Commit

Permalink
chore(ssa refactor): Remove unit values from SSA IR (#1646)
Browse files Browse the repository at this point in the history
* Remove unit values

* Fix test

* Fix comment
  • Loading branch information
jfecher authored Jun 12, 2023
1 parent e4cf984 commit bd8379e
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 29 deletions.
11 changes: 0 additions & 11 deletions crates/noirc_evaluator/src/ssa_refactor/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,14 +302,6 @@ impl Context {
_ => unreachable!("ICE: Program must have a singular return"),
};

// Check if the program returns the `Unit/None` type.
// This type signifies that the program returns nothing.
let is_return_unit_type =
return_values.len() == 1 && dfg.type_of_value(return_values[0]) == Type::Unit;
if is_return_unit_type {
return;
}

// The return value may or may not be an array reference. Calling `flatten_value_list`
// will expand the array if there is one.
let return_acir_vars = self.flatten_value_list(return_values, dfg);
Expand Down Expand Up @@ -457,9 +449,6 @@ impl Context {
(_, Type::Array(..)) | (Type::Array(..), _) => {
unreachable!("Arrays are invalid in binary operations")
}
// Unit type currently can mean a 0 constant, so we return the
// other type.
(typ, Type::Unit) | (Type::Unit, typ) => typ,
// If either side is a Field constant then, we coerce into the type
// of the other operand
(Type::Numeric(NumericType::NativeField), typ)
Expand Down
4 changes: 0 additions & 4 deletions crates/noirc_evaluator/src/ssa_refactor/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ pub(crate) enum Type {

/// A function that may be called directly
Function,

/// The Unit type with a single value
Unit,
}

impl Type {
Expand Down Expand Up @@ -78,7 +75,6 @@ impl std::fmt::Display for Type {
write!(f, "[{}; {length}]", elements.join(", "))
}
Type::Function => write!(f, "function"),
Type::Unit => write!(f, "unit"),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa_refactor/opt/mem2reg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ mod tests {
// v0 = allocate
// store v0, Field 1
// v1 = load v0
// v2 = call f0(v0)
// call f0(v0)
// return v1
// }

Expand All @@ -303,7 +303,7 @@ mod tests {
builder.insert_store(v0, one);
let v1 = builder.insert_load(v0, Type::field());
let f0 = builder.import_intrinsic_id(Intrinsic::Println);
builder.insert_call(f0, vec![v0], vec![Type::Unit]);
builder.insert_call(f0, vec![v0], vec![]);
builder.terminate_with_return(vec![v1]);

let ssa = builder.finish().mem2reg();
Expand Down
10 changes: 5 additions & 5 deletions crates/noirc_evaluator/src/ssa_refactor/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ impl<'a> FunctionContext<'a> {
ast::Type::Tuple(fields) => {
Tree::Branch(vecmap(fields, |field| Self::map_type_helper(field, f)))
}
ast::Type::Unit => Tree::empty(),
other => Tree::Leaf(f(Self::convert_non_tuple_type(other))),
}
}
Expand Down Expand Up @@ -197,7 +198,7 @@ impl<'a> FunctionContext<'a> {
ast::Type::Integer(Signedness::Unsigned, bits) => Type::unsigned(*bits),
ast::Type::Bool => Type::unsigned(1),
ast::Type::String(_) => Type::Reference,
ast::Type::Unit => Type::Unit,
ast::Type::Unit => panic!("convert_non_tuple_type called on a unit type"),
ast::Type::Tuple(_) => panic!("convert_non_tuple_type called on a tuple: {typ}"),
ast::Type::Function(_, _) => Type::Function,

Expand All @@ -208,10 +209,9 @@ impl<'a> FunctionContext<'a> {
}
}

/// Insert a unit constant into the current function if not already
/// present, and return its value
pub(super) fn unit_value(&mut self) -> Values {
self.builder.numeric_constant(0u128, Type::Unit).into()
/// Returns the unit value, represented as an empty tree of values
pub(super) fn unit_value() -> Values {
Values::empty()
}

/// Insert a binary instruction at the end of the current block.
Expand Down
14 changes: 7 additions & 7 deletions crates/noirc_evaluator/src/ssa_refactor/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl<'a> FunctionContext<'a> {
}

fn codegen_block(&mut self, block: &[Expression]) -> Values {
let mut result = self.unit_value();
let mut result = Self::unit_value();
for expr in block {
result = self.codegen_expression(expr);
}
Expand Down Expand Up @@ -258,7 +258,7 @@ impl<'a> FunctionContext<'a> {

// Finish by switching back to the end of the loop
self.builder.switch_to_block(loop_end);
self.unit_value()
Self::unit_value()
}

/// Codegens an if expression, handling the case of what to do if there is no 'else'.
Expand Down Expand Up @@ -296,7 +296,7 @@ impl<'a> FunctionContext<'a> {
self.builder.switch_to_block(then_block);
let then_value = self.codegen_expression(&if_expr.consequence);

let mut result = self.unit_value();
let mut result = Self::unit_value();

if let Some(alternative) = &if_expr.alternative {
let end_block = self.builder.insert_block();
Expand Down Expand Up @@ -363,25 +363,25 @@ impl<'a> FunctionContext<'a> {
}

self.define(let_expr.id, values);
self.unit_value()
Self::unit_value()
}

fn codegen_constrain(&mut self, expr: &Expression, _location: Location) -> Values {
let boolean = self.codegen_non_tuple_expression(expr);
self.builder.insert_constrain(boolean);
self.unit_value()
Self::unit_value()
}

fn codegen_assign(&mut self, assign: &ast::Assign) -> Values {
let lhs = self.extract_current_value(&assign.lvalue);
let rhs = self.codegen_expression(&assign.expression);

self.assign_new_value(lhs, rhs);
self.unit_value()
Self::unit_value()
}

fn codegen_semi(&mut self, expr: &Expression) -> Values {
self.codegen_expression(expr);
self.unit_value()
Self::unit_value()
}
}
5 changes: 5 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/ssa_gen/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ impl Value {
pub(super) type Values = Tree<Value>;

impl<T> Tree<T> {
/// Returns an empty tree node represented by a Branch with no branches
pub(super) fn empty() -> Self {
Tree::Branch(vec![])
}

/// Flattens the tree into a vector of each leaf value
pub(super) fn flatten(self) -> Vec<T> {
match self {
Expand Down

0 comments on commit bd8379e

Please sign in to comment.