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(ssa refactor): Add all remaining doc comments to ssa generation pass #1256

Merged
merged 5 commits into from
May 1, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 0 additions & 1 deletion crates/noirc_evaluator/src/ssa_refactor/ir.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
pub(crate) mod basic_block;
pub(crate) mod basic_block_visitors;
pub(crate) mod cfg;
pub(crate) mod constant;
pub(crate) mod dfg;
Expand Down
23 changes: 16 additions & 7 deletions crates/noirc_evaluator/src/ssa_refactor/ir/basic_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ pub(crate) struct BasicBlock {
/// Instructions in the basic block.
instructions: Vec<InstructionId>,

/// A basic block is considered sealed
/// if no further predecessors will be added to it.
/// Since only filled blocks can have successors,
/// predecessors are always filled.
is_sealed: bool,

/// The terminating instruction for the basic block.
///
/// This will be a control flow instruction. This is only
Expand All @@ -35,14 +29,20 @@ pub(crate) struct BasicBlock {
pub(crate) type BasicBlockId = Id<BasicBlock>;

impl BasicBlock {
/// Create a new BasicBlock with the given parameters.
/// Parameters can also be added later via BasicBlock::add_parameter
pub(crate) fn new(parameters: Vec<ValueId>) -> Self {
Self { parameters, instructions: Vec::new(), is_sealed: false, terminator: None }
Self { parameters, instructions: Vec::new(), terminator: None }
}

/// Returns the parameters of this block
pub(crate) fn parameters(&self) -> &[ValueId] {
&self.parameters
}

/// Adds a parameter to this BasicBlock.
/// Expects that the ValueId given should refer to a Value::Param
/// instance with its position equal to self.parameters.len().
pub(crate) fn add_parameter(&mut self, parameter: ValueId) {
self.parameters.push(parameter);
}
Expand All @@ -52,14 +52,23 @@ impl BasicBlock {
self.instructions.push(instruction);
}

/// Retrieve a reference to all instructions in this block.
pub(crate) fn instructions(&self) -> &[InstructionId] {
&self.instructions
}

/// Sets the terminator instruction of this block.
///
/// A properly-constructed block will always terminate with a TerminatorInstruction -
/// which either jumps to another block or returns from the current function. A block
/// will only have no terminator if it is still under construction.
pub(crate) fn set_terminator(&mut self, terminator: TerminatorInstruction) {
self.terminator = Some(terminator);
}

/// Returns the terminator of this block.
///
/// If this block is not still under construction, this is expected to always be Some.
jfecher marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) fn terminator(&self) -> Option<&TerminatorInstruction> {
self.terminator.as_ref()
}
Expand Down
23 changes: 0 additions & 23 deletions crates/noirc_evaluator/src/ssa_refactor/ir/basic_block_visitors.rs

This file was deleted.

57 changes: 31 additions & 26 deletions crates/noirc_evaluator/src/ssa_refactor/ir/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::collections::{HashMap, HashSet};

use super::{
basic_block::{BasicBlock, BasicBlockId},
basic_block_visitors,
function::Function,
};

Expand Down Expand Up @@ -33,25 +32,30 @@ impl ControlFlowGraph {
cfg
}

/// Compute all the edges between each block in the function
jfecher marked this conversation as resolved.
Show resolved Hide resolved
fn compute(&mut self, func: &Function) {
for (basic_block_id, basic_block) in func.dfg.basic_blocks_iter() {
self.compute_block(basic_block_id, basic_block);
}
}

/// Compute all the edges for the current block given
jfecher marked this conversation as resolved.
Show resolved Hide resolved
fn compute_block(&mut self, basic_block_id: BasicBlockId, basic_block: &BasicBlock) {
basic_block_visitors::visit_block_succs(basic_block, |dest| {
for dest in basic_block.successors() {
self.add_edge(basic_block_id, dest);
});
}
}

/// Clears out a given block's successors. This also removes the given block from
/// being a predecessor of any of its previous successors.
fn invalidate_block_successors(&mut self, basic_block_id: BasicBlockId) {
let node = self
.data
.get_mut(&basic_block_id)
.expect("ICE: Attempted to invalidate cfg node successors for non-existent node.");
let old_successors = node.successors.clone();
node.successors.clear();

let old_successors = std::mem::take(&mut node.successors);

for successor_id in old_successors {
self.data
.get_mut(&successor_id)
Expand All @@ -71,6 +75,7 @@ impl ControlFlowGraph {
self.compute_block(basic_block_id, basic_block);
}

/// Add a directed edge making `from` a predecessor of `to`.
fn add_edge(&mut self, from: BasicBlockId, to: BasicBlockId) {
let predecessor_node = self.data.entry(from).or_default();
assert!(
Expand All @@ -87,7 +92,7 @@ impl ControlFlowGraph {
}

/// Get an iterator over the CFG predecessors to `basic_block_id`.
pub(crate) fn pred_iter(
pub(crate) fn predecessors(
&self,
basic_block_id: BasicBlockId,
) -> impl ExactSizeIterator<Item = BasicBlockId> + '_ {
Expand All @@ -100,7 +105,7 @@ impl ControlFlowGraph {
}

/// Get an iterator over the CFG successors to `basic_block_id`.
pub(crate) fn succ_iter(
pub(crate) fn successors(
&self,
basic_block_id: BasicBlockId,
) -> impl ExactSizeIterator<Item = BasicBlockId> + '_ {
Expand Down Expand Up @@ -133,11 +138,11 @@ mod tests {
fn jumps() {
// Build function of form
// fn func {
// block0(cond: u1):
// block0(cond: u1):
// jmpif cond, then: block2, else: block1
// block1():
// block1():
// jmpif cond, then: block1, else: block2
// block2():
// block2():
// return ()
// }
let func_id = Id::test_new(0);
Expand All @@ -163,13 +168,13 @@ mod tests {

#[allow(clippy::needless_collect)]
{
let block0_predecessors: Vec<_> = cfg.pred_iter(block0_id).collect();
let block1_predecessors: Vec<_> = cfg.pred_iter(block1_id).collect();
let block2_predecessors: Vec<_> = cfg.pred_iter(block2_id).collect();
let block0_predecessors: Vec<_> = cfg.predecessors(block0_id).collect();
let block1_predecessors: Vec<_> = cfg.predecessors(block1_id).collect();
let block2_predecessors: Vec<_> = cfg.predecessors(block2_id).collect();

let block0_successors: Vec<_> = cfg.succ_iter(block0_id).collect();
let block1_successors: Vec<_> = cfg.succ_iter(block1_id).collect();
let block2_successors: Vec<_> = cfg.succ_iter(block2_id).collect();
let block0_successors: Vec<_> = cfg.successors(block0_id).collect();
let block1_successors: Vec<_> = cfg.successors(block1_id).collect();
let block2_successors: Vec<_> = cfg.successors(block2_id).collect();

assert_eq!(block0_predecessors.len(), 0);
assert_eq!(block1_predecessors.len(), 2);
Expand All @@ -192,13 +197,13 @@ mod tests {

// Modify function to form:
// fn func {
// block0(cond: u1):
// block0(cond: u1):
// jmpif cond, then: block1, else: ret_block
// block1():
// block1():
// jmpif cond, then: block1, else: block2
// block2():
// block2():
// jmp ret_block()
// ret_block():
// ret_block():
// return ()
// }
let ret_block_id = func.dfg.make_block();
Expand All @@ -221,13 +226,13 @@ mod tests {

#[allow(clippy::needless_collect)]
{
let block0_predecessors: Vec<_> = cfg.pred_iter(block0_id).collect();
let block1_predecessors: Vec<_> = cfg.pred_iter(block1_id).collect();
let block2_predecessors: Vec<_> = cfg.pred_iter(block2_id).collect();
let block0_predecessors: Vec<_> = cfg.predecessors(block0_id).collect();
let block1_predecessors: Vec<_> = cfg.predecessors(block1_id).collect();
let block2_predecessors: Vec<_> = cfg.predecessors(block2_id).collect();

let block0_successors: Vec<_> = cfg.succ_iter(block0_id).collect();
let block1_successors: Vec<_> = cfg.succ_iter(block1_id).collect();
let block2_successors: Vec<_> = cfg.succ_iter(block2_id).collect();
let block0_successors: Vec<_> = cfg.successors(block0_id).collect();
let block1_successors: Vec<_> = cfg.successors(block1_id).collect();
let block2_successors: Vec<_> = cfg.successors(block2_id).collect();

assert_eq!(block0_predecessors.len(), 0);
assert_eq!(block1_predecessors.len(), 2);
Expand Down
4 changes: 4 additions & 0 deletions crates/noirc_evaluator/src/ssa_refactor/ir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,21 @@ use super::map::Id;
pub(crate) struct NumericConstant(FieldElement);

impl NumericConstant {
/// Create a new NumericConstant with the given Field value
pub(crate) fn new(value: FieldElement) -> Self {
Self(value)
}

/// Retrieves the Field value for this constant
pub(crate) fn value(&self) -> FieldElement {
self.0
}
}

pub(crate) type NumericConstantId = Id<NumericConstant>;

// Implement some common numeric operations for NumericConstants
// for convenience so developers do not always have to unwrap them to use them.
impl std::ops::Add for NumericConstant {
type Output = NumericConstant;

Expand Down
40 changes: 9 additions & 31 deletions crates/noirc_evaluator/src/ssa_refactor/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,10 @@ use super::{
use acvm::FieldElement;
use iter_extended::vecmap;

#[derive(Debug, Default)]
/// A convenience wrapper to store `Value`s.
pub(crate) struct ValueList(Vec<Id<Value>>);

impl ValueList {
/// Inserts an element to the back of the list and
/// returns the `position`
pub(crate) fn push(&mut self, value: ValueId) -> usize {
self.0.push(value);
self.len() - 1
}

/// Returns the number of values in the list.
fn len(&self) -> usize {
self.0.len()
}

/// Removes all items from the list.
fn clear(&mut self) {
self.0.clear();
}

/// Returns the ValueId's as a slice.
pub(crate) fn as_slice(&self) -> &[ValueId] {
&self.0
}
}

/// The DataFlowGraph contains most of the actual data in a function including
/// its blocks, instructions, and values. This struct is largely responsible for
/// owning most data in a function and handing out Ids to this data that can be
/// shared without worrying about ownership.
#[derive(Debug, Default)]
pub(crate) struct DataFlowGraph {
/// All of the instructions in a function
Expand All @@ -57,7 +33,7 @@ pub(crate) struct DataFlowGraph {
/// Currently, we need to define them in a better way
/// Call instructions require the func signature, but
/// other instructions may need some more reading on my part
results: HashMap<InstructionId, ValueList>,
results: HashMap<InstructionId, Vec<ValueId>>,

/// Storage for all of the values defined in this
/// function.
Expand Down Expand Up @@ -243,8 +219,7 @@ impl DataFlowGraph {
});

// Add value to the list of results for this instruction
let actual_res_position = results.push(value_id);
assert_eq!(actual_res_position, expected_res_position);
results.push(value_id);
value_id
}

Expand All @@ -259,6 +234,7 @@ impl DataFlowGraph {
self.results.get(&instruction_id).expect("expected a list of Values").as_slice()
}

/// Add a parameter to the given block
pub(crate) fn add_block_parameter(&mut self, block_id: BasicBlockId, typ: Type) -> Id<Value> {
let block = &mut self.blocks[block_id];
let position = block.parameters().len();
Expand All @@ -267,6 +243,8 @@ impl DataFlowGraph {
parameter
}

/// Insert an instruction at the end of a given block.
/// If the block already has a terminator, the instruction is inserted before the terminator.
jfecher marked this conversation as resolved.
Show resolved Hide resolved
pub(crate) fn insert_instruction_in_block(
&mut self,
block: BasicBlockId,
Expand Down
Loading