Skip to content

Commit

Permalink
chore: Try replace callstack with a linked list (noir-lang#6747)
Browse files Browse the repository at this point in the history
Co-authored-by: Tom French <[email protected]>
  • Loading branch information
jfecher and TomAFrench authored Dec 11, 2024
1 parent e973397 commit f6957fa
Show file tree
Hide file tree
Showing 9 changed files with 200 additions and 13 deletions.
6 changes: 4 additions & 2 deletions compiler/noirc_evaluator/src/acir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2893,7 +2893,6 @@ mod test {
},
FieldElement,
};
use im::vector;
use noirc_errors::Location;
use noirc_frontend::monomorphization::ast::InlineType;
use std::collections::BTreeMap;
Expand All @@ -2904,6 +2903,7 @@ mod test {
ssa::{
function_builder::FunctionBuilder,
ir::{
dfg::CallStack,
function::FunctionId,
instruction::BinaryOp,
map::Id,
Expand All @@ -2930,7 +2930,9 @@ mod test {
builder.new_function("foo".into(), foo_id, inline_type);
}
// Set a call stack for testing whether `brillig_locations` in the `GeneratedAcir` was accurately set.
builder.set_call_stack(vector![Location::dummy(), Location::dummy()]);
let mut stack = CallStack::unit(Location::dummy());
stack.push_back(Location::dummy());
builder.set_call_stack(stack);

let foo_v0 = builder.add_parameter(Type::field());
let foo_v1 = builder.add_parameter(Type::field());
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 @@ -196,7 +196,7 @@ impl FunctionBuilder {
}

pub(crate) fn set_location(&mut self, location: Location) -> &mut FunctionBuilder {
self.call_stack = im::Vector::unit(location);
self.call_stack = CallStack::unit(location);
self
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/ir/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ mod tests {
func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp {
destination: ret_block_id,
arguments: vec![],
call_stack: im::Vector::new(),
call_stack: CallStack::new(),
});
func.dfg[block0_id].set_terminator(TerminatorInstruction::JmpIf {
condition: cond,
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ pub(crate) struct DataFlowGraph {
pub(crate) data_bus: DataBus,
}

pub(crate) type CallStack = im::Vector<Location>;
pub(crate) type CallStack = super::list::List<Location>;

impl DataFlowGraph {
/// Creates a new basic block with no parameters.
Expand Down Expand Up @@ -497,7 +497,7 @@ impl DataFlowGraph {
pub(crate) fn get_value_call_stack(&self, value: ValueId) -> CallStack {
match &self.values[self.resolve(value)] {
Value::Instruction { instruction, .. } => self.get_call_stack(*instruction),
_ => im::Vector::new(),
_ => CallStack::new(),
}
}

Expand Down
3 changes: 1 addition & 2 deletions compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use noirc_errors::Location;
use serde::{Deserialize, Serialize};
use std::hash::{Hash, Hasher};

Expand Down Expand Up @@ -1248,7 +1247,7 @@ impl TerminatorInstruction {
}
}

pub(crate) fn call_stack(&self) -> im::Vector<Location> {
pub(crate) fn call_stack(&self) -> CallStack {
match self {
TerminatorInstruction::JmpIf { call_stack, .. }
| TerminatorInstruction::Jmp { call_stack, .. }
Expand Down
187 changes: 187 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/list.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
use serde::{Deserialize, Serialize};
use std::sync::Arc;

/// A shared linked list type intended to be cloned
#[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
pub struct List<T> {
head: Arc<Node<T>>,
len: usize,
}

#[derive(Default, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
enum Node<T> {
#[default]
Nil,
Cons(T, Arc<Node<T>>),
}

impl<T> Default for List<T> {
fn default() -> Self {
List { head: Arc::new(Node::Nil), len: 0 }
}
}

impl<T> List<T> {
pub fn new() -> Self {
Self::default()
}

/// This is actually a push_front since we just create a new head for the
/// list. This is done so that the tail of the list can still be shared.
/// In the case of call stacks, the last node will be main, while the top
/// of the call stack will be the head of this list.
pub fn push_back(&mut self, value: T) {
self.len += 1;
self.head = Arc::new(Node::Cons(value, self.head.clone()));
}

/// It is more efficient to iterate from the head of the list towards the tail.
/// For callstacks this means from the top of the call stack towards main.
fn iter_rev(&self) -> IterRev<T> {
IterRev { head: &self.head, len: self.len }
}

pub fn clear(&mut self) {
*self = Self::default();
}

pub fn append(&mut self, other: Self)
where
T: Copy + std::fmt::Debug,
{
let other = other.into_iter().collect::<Vec<_>>();

for item in other {
self.push_back(item);
}
}

pub fn len(&self) -> usize {
self.len
}

pub fn is_empty(&self) -> bool {
self.len == 0
}

fn pop_front(&mut self) -> Option<T>
where
T: Copy,
{
match self.head.as_ref() {
Node::Nil => None,
Node::Cons(value, rest) => {
let value = *value;
self.head = rest.clone();
self.len -= 1;
Some(value)
}
}
}

pub fn truncate(&mut self, len: usize)
where
T: Copy,
{
if self.len > len {
for _ in 0..self.len - len {
self.pop_front();
}
}
}

pub fn unit(item: T) -> Self {
let mut this = Self::default();
this.push_back(item);
this
}

pub fn back(&self) -> Option<&T> {
match self.head.as_ref() {
Node::Nil => None,
Node::Cons(item, _) => Some(item),
}
}
}

pub struct IterRev<'a, T> {
head: &'a Node<T>,
len: usize,
}

impl<T> IntoIterator for List<T>
where
T: Copy + std::fmt::Debug,
{
type Item = T;

type IntoIter = std::iter::Rev<std::vec::IntoIter<T>>;

fn into_iter(self) -> Self::IntoIter {
let items: Vec<_> = self.iter_rev().copied().collect();
items.into_iter().rev()
}
}

impl<'a, T> IntoIterator for &'a List<T> {
type Item = &'a T;

type IntoIter = std::iter::Rev<<Vec<&'a T> as IntoIterator>::IntoIter>;

fn into_iter(self) -> Self::IntoIter {
let items: Vec<_> = self.iter_rev().collect();
items.into_iter().rev()
}
}

impl<'a, T> Iterator for IterRev<'a, T> {
type Item = &'a T;

fn next(&mut self) -> Option<Self::Item> {
match self.head {
Node::Nil => None,
Node::Cons(value, rest) => {
self.head = rest;
Some(value)
}
}
}

fn size_hint(&self) -> (usize, Option<usize>) {
(0, Some(self.len))
}
}

impl<'a, T> ExactSizeIterator for IterRev<'a, T> {}

impl<T> std::fmt::Debug for List<T>
where
T: std::fmt::Debug,
{
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "[")?;
for (i, item) in self.iter_rev().enumerate() {
if i != 0 {
write!(f, ", ")?;
}
write!(f, "{item:?}")?;
}
write!(f, "]")
}
}

impl<T> std::fmt::Display for List<T>
where
T: std::fmt::Display,
{
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "[")?;
for (i, item) in self.iter_rev().enumerate() {
if i != 0 {
write!(f, ", ")?;
}
write!(f, "{item}")?;
}
write!(f, "]")
}
}
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/src/ssa/ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub(crate) mod dom;
pub(crate) mod function;
pub(crate) mod function_inserter;
pub(crate) mod instruction;
pub mod list;
pub(crate) mod map;
pub(crate) mod post_order;
pub(crate) mod printer;
Expand Down
6 changes: 2 additions & 4 deletions compiler/noirc_evaluator/src/ssa/opt/die.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
//! Dead Instruction Elimination (DIE) pass: Removes any instruction without side-effects for
//! which the results are unused.
use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};
use im::Vector;
use noirc_errors::Location;
use rayon::iter::{IntoParallelRefMutIterator, ParallelIterator};

use crate::ssa::{
ir::{
basic_block::{BasicBlock, BasicBlockId},
dfg::DataFlowGraph,
dfg::{CallStack, DataFlowGraph},
function::Function,
instruction::{BinaryOp, Instruction, InstructionId, Intrinsic},
post_order::PostOrder,
Expand Down Expand Up @@ -484,7 +482,7 @@ fn apply_side_effects(
rhs: ValueId,
function: &mut Function,
block_id: BasicBlockId,
call_stack: Vector<Location>,
call_stack: CallStack,
) -> (ValueId, ValueId) {
// See if there's an active "enable side effects" condition
let Some(condition) = side_effects_condition else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ impl<'f> Context<'f> {
self.insert_instruction_with_typevars(
Instruction::EnableSideEffectsIf { condition: one },
None,
im::Vector::new(),
CallStack::new(),
);
self.push_instruction(*instruction);
self.insert_current_side_effects_enabled();
Expand Down

0 comments on commit f6957fa

Please sign in to comment.