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: Rework DIE pass to operate on a postorder SCC CFG #6490

Closed
wants to merge 7 commits into from
Closed
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
Next Next commit
Start reworking die to work on SCCs
jfecher committed Nov 8, 2024
commit 8ed5b52256437849e193b4f832a94226be286833
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -163,6 +163,7 @@ tracing = "0.1.40"
tracing-web = "0.1.3"
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
rust-embed = "6.6.0"
petgraph = "0.6"

[profile.dev]
# This is required to be able to run `cargo test` in acvm_js due to the `locals exceeds maximum` error.
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/Cargo.toml
Original file line number Diff line number Diff line change
@@ -28,6 +28,7 @@ tracing.workspace = true
chrono = "0.4.37"
rayon.workspace = true
cfg-if.workspace = true
petgraph.workspace = true

[dev-dependencies]
proptest.workspace = true
44 changes: 44 additions & 0 deletions compiler/noirc_evaluator/src/ssa/ir/function.rs
Original file line number Diff line number Diff line change
@@ -2,8 +2,12 @@ use std::collections::BTreeSet;

use iter_extended::vecmap;
use noirc_frontend::monomorphization::ast::InlineType;
use petgraph::graph::NodeIndex;
use petgraph::prelude::DiGraph;
use serde::{Deserialize, Serialize};

use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet};

use super::basic_block::BasicBlockId;
use super::dfg::DataFlowGraph;
use super::instruction::TerminatorInstruction;
@@ -187,6 +191,46 @@ impl Function {

unreachable!("SSA Function {} has no reachable return instruction!", self.id())
}

/// Return each SCC (strongly-connected component) of the CFG of this function
/// in postorder. In practice this is identical to a normal CFG except that
/// blocks within loops will all be grouped together in the same SCC.
pub(crate) fn postorder_scc_cfg(&self) -> Vec<Vec<BasicBlockId>> {
let mut cfg = DiGraph::new();
let mut stack = vec![self.entry_block];

let mut visited = HashSet::default();
let mut block_to_index = HashMap::default();
let mut index_to_block = HashMap::default();

// Add or create a block node in the cfg
let mut get_block_index = |cfg: &mut DiGraph<_, _>, block| {
block_to_index.get(&block).copied().unwrap_or_else(|| {
let index = cfg.add_node(block);
block_to_index.insert(block, index);
index_to_block.insert(index, block);
index
})
};

// Populate each reachable block & edges between them
while let Some(block) = stack.pop() {
if visited.insert(block) {
let block_index = get_block_index(&mut cfg, block);

for successor in self.dfg[block].successors() {
stack.push(successor);
let successor_index = get_block_index(&mut cfg, successor);
cfg.add_edge(block_index, successor_index, ());
}
}
}

// Perform tarjan_scc to get strongly connected components.
// Lucky for us, this already returns SCCs in postorder.
let postorder: Vec<Vec<NodeIndex>> = petgraph::algo::tarjan_scc(&cfg);
vecmap(postorder, |indices| vecmap(indices, |index| index_to_block[&index]))
}
}

impl std::fmt::Display for RuntimeType {
66 changes: 36 additions & 30 deletions compiler/noirc_evaluator/src/ssa/ir/post_order.rs
Original file line number Diff line number Diff line change
@@ -15,6 +15,12 @@ enum Visit {

pub(crate) struct PostOrder(Vec<BasicBlockId>);

/// An SCC Post Order is a post-order visit of each strongly-connected
/// component in a function's control flow graph. In practice this is
/// identical to a normal post order except that blocks within loops
/// will all be grouped together within the same visit step.
pub(crate) struct SccPostOrder(Vec<Vec<BasicBlockId>>);

impl PostOrder {
pub(crate) fn as_slice(&self) -> &[BasicBlockId] {
self.0.as_slice()
@@ -24,49 +30,49 @@ impl PostOrder {
impl PostOrder {
/// Allocate and compute a function's block post-order. Pos
pub(crate) fn with_function(func: &Function) -> Self {
PostOrder(Self::compute_post_order(func))
PostOrder(compute_post_order(func))
}

pub(crate) fn into_vec(self) -> Vec<BasicBlockId> {
self.0
}
}

// Computes the post-order of the function by doing a depth-first traversal of the
// function's entry block's previously unvisited children. Each block is sequenced according
// to when the traversal exits it.
fn compute_post_order(func: &Function) -> Vec<BasicBlockId> {
let mut stack = vec![(Visit::First, func.entry_block())];
let mut visited: HashSet<BasicBlockId> = HashSet::new();
let mut post_order: Vec<BasicBlockId> = Vec::new();

while let Some((visit, block_id)) = stack.pop() {
match visit {
Visit::First => {
if !visited.contains(&block_id) {
// This is the first time we pop the block, so we need to scan its
// successors and then revisit it.
visited.insert(block_id);
stack.push((Visit::Last, block_id));
// Stack successors for visiting. Because items are taken from the top of the
// stack, we push the item that's due for a visit first to the top.
for successor_id in func.dfg[block_id].successors().rev() {
if !visited.contains(&successor_id) {
// This not visited check would also be cover by the next
// iteration, but checking here two saves an iteration per successor.
stack.push((Visit::First, successor_id));
}
// Computes the post-order of the function by doing a depth-first traversal of the
// function's entry block's previously unvisited children. Each block is sequenced according
// to when the traversal exits it.
fn compute_post_order(func: &Function) -> Vec<BasicBlockId> {
let mut stack = vec![(Visit::First, func.entry_block())];
let mut visited: HashSet<BasicBlockId> = HashSet::new();
let mut post_order: Vec<BasicBlockId> = Vec::new();

while let Some((visit, block_id)) = stack.pop() {
match visit {
Visit::First => {
if !visited.contains(&block_id) {
// This is the first time we pop the block, so we need to scan its
// successors and then revisit it.
visited.insert(block_id);
stack.push((Visit::Last, block_id));
// Stack successors for visiting. Because items are taken from the top of the
// stack, we push the item that's due for a visit first to the top.
for successor_id in func.dfg[block_id].successors() {
if !visited.contains(&successor_id) {
// This not visited check would also be cover by the next
// iteration, but checking here two saves an iteration per successor.
stack.push((Visit::First, successor_id));
}
}
}
}

Visit::Last => {
// We've finished all this node's successors.
post_order.push(block_id);
}
Visit::Last => {
// We've finished all this node's successors.
post_order.push(block_id);
}
}
post_order
}
post_order
}

#[cfg(test)]
52 changes: 40 additions & 12 deletions compiler/noirc_evaluator/src/ssa/opt/die.rs
Original file line number Diff line number Diff line change
@@ -44,15 +44,19 @@ impl Function {
context.mark_used_instruction_results(&self.dfg, call_data.array_id);
}

let postorder_scc_cfg = self.postorder_scc_cfg();
let mut inserted_out_of_bounds_checks = false;

let blocks = PostOrder::with_function(self);
for block in blocks.as_slice() {
inserted_out_of_bounds_checks |= context.remove_unused_instructions_in_block(
self,
*block,
insert_out_of_bounds_checks,
);
for scc in postorder_scc_cfg {
if scc.len() == 1 {
inserted_out_of_bounds_checks |= context.scan_and_remove_unused_instructions_in_block(
self,
scc[0],
insert_out_of_bounds_checks,
);
} else {
context.die_in_scc(scc, self);
}
}

// If we inserted out of bounds check, let's run the pass again with those new
@@ -97,7 +101,7 @@ impl Context {
/// removing unused instructions and return `true`. The idea then is to later call this
/// function again with `insert_out_of_bounds_checks` set to false to effectively remove
/// unused instructions but leave the out of bounds checks.
fn remove_unused_instructions_in_block(
fn scan_and_remove_unused_instructions_in_block(
&mut self,
function: &mut Function,
block_id: BasicBlockId,
@@ -158,13 +162,37 @@ impl Context {
}
}

function.dfg[block_id]
.instructions_mut()
.retain(|instruction| !self.instructions_to_remove.contains(instruction));

self.remove_unused_instructions_in_block(function, block_id);
false
}

fn die_in_scc(&mut self, scc: Vec<BasicBlockId>, function: &mut Function) {
let mut used_values_len = self.used_values.len();

// Continue to mark used values while we have at least one change
while {
for block in &scc {
self.mark_used_values(*block, function);
}
let len_has_changed = self.used_values.len() != used_values_len;
used_values_len = self.used_values.len();
len_has_changed
} {}

for block in scc {
// remove each unused instruction
self.remove_unused_instructions_in_block(function, block);
}
}

fn remove_unused_instructions_in_block(&self, function: &mut Function, block: BasicBlockId) {
let instructions = function.dfg[block].instructions_mut();
instructions.retain(|instruction| !self.instructions_to_remove.contains(instruction));
}

fn mark_used_values(&mut self, block_id: BasicBlockId, function: &mut Function) {
}

/// Returns true if an instruction can be removed.
///
/// An instruction can be removed as long as it has no side-effects, and none of its result
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/Cargo.toml
Original file line number Diff line number Diff line change
@@ -28,7 +28,7 @@ small-ord-set = "0.1.3"
regex = "1.9.1"
cfg-if.workspace = true
tracing.workspace = true
petgraph = "0.6"
petgraph.workspace = true
rangemap = "1.4.0"
strum = "0.24"
strum_macros = "0.24"