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: enum dummy ID for FuncId in monomorphizer and docstring fixes #5421

Merged
merged 12 commits into from
Aug 26, 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
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ impl<'block> BrilligBlock<'block> {
1,
);
}
Instruction::EnableSideEffects { .. } => {
Instruction::EnableSideEffectsIf { .. } => {
todo!("enable_side_effects not supported by brillig")
}
Instruction::IfElse { .. } => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub(crate) fn directive_invert<F: AcirField>() -> GeneratedBrillig<F> {
///
/// This is equivalent to the Noir (pseudo)code
///
/// ```ignore
/// ```text
/// fn quotient<T>(a: T, b: T) -> (T,T) {
/// (a/b, a-a/b*b)
/// }
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub(crate) fn optimize_into_acir(
.run_pass(Ssa::inline_functions_with_no_predicates, "After Inlining:")
.run_pass(Ssa::remove_if_else, "After Remove IfElse:")
.run_pass(Ssa::fold_constants, "After Constant Folding:")
.run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffects removal:")
.run_pass(Ssa::remove_enable_side_effects, "After EnableSideEffectsIf removal:")
.run_pass(Ssa::fold_constants_using_constraints, "After Constraint Folding:")
.run_pass(Ssa::dead_instruction_elimination, "After Dead Instruction Elimination:")
.run_pass(Ssa::array_set_optimization, "After Array Set Optimizations:")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ impl<F: AcirField> AcirContext<F> {
self.sub_var(sum, mul)
} else {
// Implement OR in terms of AND
// (NOT a) NAND (NOT b) => a OR b
// (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())?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ impl<'a> Context<'a> {
self.convert_ssa_truncate(*value, *bit_size, *max_bit_size, dfg)?;
self.define_result_var(dfg, instruction_id, result_acir_var);
}
Instruction::EnableSideEffects { condition } => {
Instruction::EnableSideEffectsIf { condition } => {
let acir_var = self.convert_numeric_value(*condition, dfg)?;
self.current_side_effects_enabled_var = acir_var;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ impl Context {
}
Instruction::Allocate { .. }
| Instruction::DecrementRc { .. }
| Instruction::EnableSideEffects { .. }
| Instruction::EnableSideEffectsIf { .. }
| Instruction::IncrementRc { .. }
| Instruction::RangeCheck { .. } => {}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/function_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ impl FunctionBuilder {
/// Insert an enable_side_effects_if instruction. These are normally only automatically
/// inserted during the flattening pass when branching is removed.
pub(crate) fn insert_enable_side_effects_if(&mut self, condition: ValueId) {
self.insert_instruction(Instruction::EnableSideEffects { condition }, None);
self.insert_instruction(Instruction::EnableSideEffectsIf { condition }, None);
}

/// Terminates the current block with the given terminator instruction
Expand Down
45 changes: 33 additions & 12 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,23 @@ pub(crate) enum Instruction {
Store { address: ValueId, value: ValueId },

/// Provides a context for all instructions that follow up until the next
/// `EnableSideEffects` is encountered, for stating a condition that determines whether
/// `EnableSideEffectsIf` is encountered, for stating a condition that determines whether
/// such instructions are allowed to have side-effects.
///
/// For example,
/// ```text
/// EnableSideEffectsIf condition0;
/// code0;
/// EnableSideEffectsIf condition1;
/// code1;
/// ```
/// - `code0` will have side effects iff `condition0` evaluates to `true`
/// - `code1` will have side effects iff `condition1` evaluates to `true`
///
/// This instruction is only emitted after the cfg flattening pass, and is used to annotate
/// instruction regions with an condition that corresponds to their position in the CFG's
/// if-branching structure.
EnableSideEffects { condition: ValueId },
EnableSideEffectsIf { condition: ValueId },

/// Retrieve a value from an array at the given index
ArrayGet { array: ValueId, index: ValueId },
Expand All @@ -249,6 +259,17 @@ pub(crate) enum Instruction {
DecrementRc { value: ValueId },

/// Merge two values returned from opposite branches of a conditional into one.
///
/// ```text
/// if then_condition {
/// then_value
/// } else { // else_condition = !then_condition
/// else_value
/// }
/// ```
///
/// Where we save the result of !then_condition so that we have the same
/// ValueId for it each time.
IfElse {
then_condition: ValueId,
then_value: ValueId,
Expand Down Expand Up @@ -279,7 +300,7 @@ impl Instruction {
| Instruction::IncrementRc { .. }
| Instruction::DecrementRc { .. }
| Instruction::RangeCheck { .. }
| Instruction::EnableSideEffects { .. } => InstructionResultType::None,
| Instruction::EnableSideEffectsIf { .. } => InstructionResultType::None,
Instruction::Allocate { .. }
| Instruction::Load { .. }
| Instruction::ArrayGet { .. }
Expand All @@ -306,7 +327,7 @@ impl Instruction {

match self {
// These either have side-effects or interact with memory
EnableSideEffects { .. }
EnableSideEffectsIf { .. }
| Allocate
| Load { .. }
| Store { .. }
Expand Down Expand Up @@ -362,7 +383,7 @@ impl Instruction {

Constrain(..)
| Store { .. }
| EnableSideEffects { .. }
| EnableSideEffectsIf { .. }
| IncrementRc { .. }
| DecrementRc { .. }
| RangeCheck { .. } => false,
Expand Down Expand Up @@ -401,7 +422,7 @@ impl Instruction {
!dfg.is_safe_index(*index, *array)
}

Instruction::EnableSideEffects { .. } | Instruction::ArraySet { .. } => true,
Instruction::EnableSideEffectsIf { .. } | Instruction::ArraySet { .. } => true,

Instruction::Call { func, .. } => match dfg[*func] {
Value::Function(_) => true,
Expand Down Expand Up @@ -466,8 +487,8 @@ impl Instruction {
Instruction::Store { address, value } => {
Instruction::Store { address: f(*address), value: f(*value) }
}
Instruction::EnableSideEffects { condition } => {
Instruction::EnableSideEffects { condition: f(*condition) }
Instruction::EnableSideEffectsIf { condition } => {
Instruction::EnableSideEffectsIf { condition: f(*condition) }
}
Instruction::ArrayGet { array, index } => {
Instruction::ArrayGet { array: f(*array), index: f(*index) }
Expand Down Expand Up @@ -541,7 +562,7 @@ impl Instruction {
f(*index);
f(*value);
}
Instruction::EnableSideEffects { condition } => {
Instruction::EnableSideEffectsIf { condition } => {
f(*condition);
}
Instruction::IncrementRc { value }
Expand Down Expand Up @@ -678,11 +699,11 @@ impl Instruction {
Instruction::Call { func, arguments } => {
simplify_call(*func, arguments, dfg, block, ctrl_typevars, call_stack)
}
Instruction::EnableSideEffects { condition } => {
Instruction::EnableSideEffectsIf { condition } => {
if let Some(last) = dfg[block].instructions().last().copied() {
let last = &mut dfg[last];
if matches!(last, Instruction::EnableSideEffects { .. }) {
*last = Instruction::EnableSideEffects { condition: *condition };
if matches!(last, Instruction::EnableSideEffectsIf { .. }) {
*last = Instruction::EnableSideEffectsIf { condition: *condition };
return Remove;
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ fn display_instruction_inner(
Instruction::Store { address, value } => {
writeln!(f, "store {} at {}", show(*value), show(*address))
}
Instruction::EnableSideEffects { condition } => {
Instruction::EnableSideEffectsIf { condition } => {
writeln!(f, "enable_side_effects {}", show(*condition))
}
Instruction::ArrayGet { array, index } => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,9 +159,9 @@ impl Context {
*side_effects_enabled_var,
);

// If we just inserted an `Instruction::EnableSideEffects`, we need to update `side_effects_enabled_var`
// If we just inserted an `Instruction::EnableSideEffectsIf`, we need to update `side_effects_enabled_var`
// so that we use the correct set of constrained values in future.
if let Instruction::EnableSideEffects { condition } = instruction {
if let Instruction::EnableSideEffectsIf { condition } = instruction {
*side_effects_enabled_var = condition;
};
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/die.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ impl Context {
let instruction_id = *instruction_id;
let instruction = &function.dfg[instruction_id];

if let Instruction::EnableSideEffects { condition } = instruction {
if let Instruction::EnableSideEffectsIf { condition } = instruction {
side_effects_condition = Some(*condition);

// We still need to keep the EnableSideEffects instruction
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//! elimination (DIE) pass.
//!
//! Though CFG information is lost during this pass, some key information is retained in the form
//! of `EnableSideEffect` instructions. Each time the flattening pass enters and exits a branch of
//! of `EnableSideEffectsIf` instructions. Each time the flattening pass enters and exits a branch of
//! a jmpif, an instruction is inserted to capture a condition that is analogous to the activeness
//! of the program point. For example:
//!
Expand Down Expand Up @@ -573,7 +573,7 @@ impl<'f> Context<'f> {
}

/// Checks the branch condition on the top of the stack and uses it to build and insert an
/// `EnableSideEffects` instruction into the entry block.
/// `EnableSideEffectsIf` instruction into the entry block.
///
/// If the stack is empty, a "true" u1 constant is taken to be the active condition. This is
/// necessary for re-enabling side-effects when re-emerging to a branch depth of 0.
Expand All @@ -584,7 +584,7 @@ impl<'f> Context<'f> {
self.inserter.function.dfg.make_constant(FieldElement::one(), Type::unsigned(1))
}
};
let enable_side_effects = Instruction::EnableSideEffects { condition };
let enable_side_effects = Instruction::EnableSideEffectsIf { condition };
let call_stack = self.inserter.function.dfg.get_value_call_stack(condition);
self.insert_instruction_with_typevars(enable_side_effects, None, call_stack);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ impl<'a> ValueMerger<'a> {
for (index, element_type, condition) in changed_indices {
let typevars = Some(vec![element_type.clone()]);

let instruction = Instruction::EnableSideEffects { condition };
let instruction = Instruction::EnableSideEffectsIf { condition };
self.insert_instruction(instruction);

let mut get_element = |array, typevars| {
Expand All @@ -398,7 +398,7 @@ impl<'a> ValueMerger<'a> {
array = self.insert_array_set(array, index, value, Some(condition)).first();
}

let instruction = Instruction::EnableSideEffects { condition: current_condition };
let instruction = Instruction::EnableSideEffectsIf { condition: current_condition };
self.insert_instruction(instruction);
Some(array)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ impl<'function> PerFunctionContext<'function> {
}
None => self.push_instruction(*id),
},
Instruction::EnableSideEffects { condition } => {
Instruction::EnableSideEffectsIf { condition } => {
side_effects_enabled = Some(self.translate_value(*condition));
self.push_instruction(*id);
}
Expand Down
20 changes: 10 additions & 10 deletions compiler/noirc_evaluator/src/ssa/opt/remove_enable_side_effects.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
//! The goal of the "remove enable side effects" optimization pass is to delay any [Instruction::EnableSideEffects]
//! The goal of the "remove enable side effects" optimization pass is to delay any [Instruction::EnableSideEffectsIf]
//! instructions such that they cover the minimum number of instructions possible.
//!
//! The pass works as follows:
//! - Insert instructions until an [Instruction::EnableSideEffects] is encountered, save this [InstructionId].
//! - Insert instructions until an [Instruction::EnableSideEffectsIf] is encountered, save this [InstructionId].
//! - Continue inserting instructions until either
//! - Another [Instruction::EnableSideEffects] is encountered, if so then drop the previous [InstructionId] in favour
//! - Another [Instruction::EnableSideEffectsIf] is encountered, if so then drop the previous [InstructionId] in favour
//! of this one.
//! - An [Instruction] with side-effects is encountered, if so then insert the currently saved [Instruction::EnableSideEffects]
//! before the [Instruction]. Continue inserting instructions until the next [Instruction::EnableSideEffects] is encountered.
//! - An [Instruction] with side-effects is encountered, if so then insert the currently saved [Instruction::EnableSideEffectsIf]
//! before the [Instruction]. Continue inserting instructions until the next [Instruction::EnableSideEffectsIf] is encountered.
use std::collections::HashSet;

use acvm::{acir::AcirField, FieldElement};
Expand Down Expand Up @@ -70,10 +70,10 @@ impl Context {
for instruction_id in instructions {
let instruction = &function.dfg[instruction_id];

// If we run into another `Instruction::EnableSideEffects` before encountering any
// If we run into another `Instruction::EnableSideEffectsIf` before encountering any
// instructions with side effects then we can drop the instruction we're holding and
// continue with the new `Instruction::EnableSideEffects`.
if let Instruction::EnableSideEffects { condition } = instruction {
// continue with the new `Instruction::EnableSideEffectsIf`.
if let Instruction::EnableSideEffectsIf { condition } = instruction {
// If this instruction isn't changing the currently active condition then we can ignore it.
if active_condition == *condition {
continue;
Expand All @@ -98,7 +98,7 @@ impl Context {
}

// If we hit an instruction which is affected by the side effects var then we must insert the
// `Instruction::EnableSideEffects` before we insert this new instruction.
// `Instruction::EnableSideEffectsIf` before we insert this new instruction.
if Self::responds_to_side_effects_var(&function.dfg, instruction) {
if let Some(enable_side_effects_instruction_id) =
last_side_effects_enabled_instruction.take()
Expand Down Expand Up @@ -140,7 +140,7 @@ impl Context {
| IncrementRc { .. }
| DecrementRc { .. } => false,

EnableSideEffects { .. }
EnableSideEffectsIf { .. }
| ArrayGet { .. }
| ArraySet { .. }
| Allocate
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/remove_if_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl Context {
self.slice_sizes.insert(result, old_capacity);
function.dfg[block].instructions_mut().push(instruction);
}
Instruction::EnableSideEffects { condition } => {
Instruction::EnableSideEffectsIf { condition } => {
current_conditional = *condition;
function.dfg[block].instructions_mut().push(instruction);
}
Expand Down
18 changes: 11 additions & 7 deletions compiler/noirc_frontend/src/monomorphization/ast.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::fmt::Display;

use acvm::FieldElement;
use iter_extended::vecmap;
use noirc_errors::{
Expand Down Expand Up @@ -67,6 +69,12 @@ pub struct LocalId(pub u32);
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct FuncId(pub u32);

impl Display for FuncId {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "{}", self.0)
}
}

#[derive(Debug, Clone, Hash)]
pub struct Ident {
pub location: Option<Location>,
Expand Down Expand Up @@ -357,16 +365,12 @@ impl Program {
FuncId(0)
}

pub fn take_main_body(&mut self) -> Expression {
self.take_function_body(FuncId(0))
}

/// Takes a function body by replacing it with `false` and
/// returning the previous value
pub fn take_function_body(&mut self, function: FuncId) -> Expression {
let main = &mut self.functions[function.0 as usize];
let replacement = Expression::Literal(Literal::Bool(false));
std::mem::replace(&mut main.body, replacement)
let function_definition = &mut self[function];
let replacement = Expression::Block(vec![]);
std::mem::replace(&mut function_definition.body, replacement)
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/monomorphization/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl AstPrinter {
write!(
f,
"fn {}$f{}({}) -> {} {{",
function.name, function.id.0, params, function.return_type
function.name, function.id, params, function.return_type
)?;
self.indent_level += 1;
self.print_expr_expect_block(&function.body, f)?;
Expand Down Expand Up @@ -291,7 +291,7 @@ impl Display for Definition {
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
match self {
Definition::Local(id) => write!(f, "l{}", id.0),
Definition::Function(id) => write!(f, "f{}", id.0),
Definition::Function(id) => write!(f, "f{}", id),
Definition::Builtin(name) => write!(f, "{name}"),
Definition::LowLevel(name) => write!(f, "{name}"),
Definition::Oracle(name) => write!(f, "{name}"),
Expand Down
Loading