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

refactor(cfg): use IndexVec for storing basic blocks #6323

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
2 changes: 2 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions crates/oxc_cfg/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ test = false
doctest = false

[dependencies]
oxc_index = { workspace = true }
oxc_syntax = { workspace = true }

bitflags = { workspace = true }
itertools = { workspace = true }
nonmax = { workspace = true }
petgraph = { workspace = true }
rustc-hash = { workspace = true }
3 changes: 0 additions & 3 deletions crates/oxc_cfg/src/block.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use oxc_syntax::node::NodeId;
use petgraph::stable_graph::NodeIndex;

pub type BasicBlockId = NodeIndex;

#[derive(Debug, Clone)]
pub struct BasicBlock {
Expand Down
36 changes: 18 additions & 18 deletions crates/oxc_cfg/src/builder/context.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::ControlFlowGraphBuilder;
use crate::{BasicBlockId, EdgeType};
use crate::{BlockNodeId, EdgeType};

bitflags::bitflags! {
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand All @@ -15,9 +15,9 @@ bitflags::bitflags! {
pub(super) struct Ctx<'a> {
flags: CtxFlags,
label: Option<&'a str>,
entries: Vec<(CtxFlags, BasicBlockId)>,
break_jmp: Option<BasicBlockId>,
continue_jmp: Option<BasicBlockId>,
entries: Vec<(CtxFlags, BlockNodeId)>,
break_jmp: Option<BlockNodeId>,
continue_jmp: Option<BlockNodeId>,
}

impl<'a> Ctx<'a> {
Expand All @@ -29,54 +29,54 @@ impl<'a> Ctx<'a> {
self.label.as_ref().is_some_and(|it| *it == label)
}

fn r#break(&mut self, entry: BasicBlockId) {
fn r#break(&mut self, entry: BlockNodeId) {
self.entries.push((CtxFlags::BREAK, entry));
}

fn r#continue(&mut self, entry: BasicBlockId) {
fn r#continue(&mut self, entry: BlockNodeId) {
self.entries.push((CtxFlags::CONTINUE, entry));
}
}

pub trait CtxCursor {
#![allow(clippy::return_self_not_must_use)]
/// Marks the break jump position in the current context.
fn mark_break(self, jmp_pos: BasicBlockId) -> Self;
fn mark_break(self, jmp_pos: BlockNodeId) -> Self;
/// Marks the continue jump position in the current context.
fn mark_continue(self, jmp_pos: BasicBlockId) -> Self;
fn mark_continue(self, jmp_pos: BlockNodeId) -> Self;
/// Creates a break entry in the current context.
fn r#break(self, bb: BasicBlockId) -> Self;
fn r#break(self, bb: BlockNodeId) -> Self;
/// Creates a continue entry in the current context.
fn r#continue(self, bb: BasicBlockId) -> Self;
fn r#continue(self, bb: BlockNodeId) -> Self;
}

pub struct QueryCtx<'a, 'c>(&'c mut ControlFlowGraphBuilder<'a>, /* label */ Option<&'a str>);

impl<'a, 'c> CtxCursor for QueryCtx<'a, 'c> {
fn mark_break(self, jmp_pos: BasicBlockId) -> Self {
fn mark_break(self, jmp_pos: BlockNodeId) -> Self {
self.0.in_break_context(self.1, |ctx| {
debug_assert!(ctx.break_jmp.is_none());
ctx.break_jmp = Some(jmp_pos);
});
self
}

fn mark_continue(self, jmp_pos: BasicBlockId) -> Self {
fn mark_continue(self, jmp_pos: BlockNodeId) -> Self {
self.0.in_continue_context(self.1, |ctx| {
debug_assert!(ctx.continue_jmp.is_none());
ctx.continue_jmp = Some(jmp_pos);
});
self
}

fn r#break(self, bb: BasicBlockId) -> Self {
fn r#break(self, bb: BlockNodeId) -> Self {
self.0.in_break_context(self.1, |ctx| {
ctx.r#break(bb);
});
self
}

fn r#continue(self, bb: BasicBlockId) -> Self {
fn r#continue(self, bb: BlockNodeId) -> Self {
self.0.in_continue_context(self.1, |ctx| {
ctx.r#continue(bb);
});
Expand Down Expand Up @@ -192,24 +192,24 @@ impl<'a, 'c> RefCtxCursor<'a, 'c> {
}

impl<'a, 'c> CtxCursor for RefCtxCursor<'a, 'c> {
fn mark_break(self, jmp_pos: BasicBlockId) -> Self {
fn mark_break(self, jmp_pos: BlockNodeId) -> Self {
debug_assert!(self.0.break_jmp.is_none());
self.0.break_jmp = Some(jmp_pos);
self
}

fn mark_continue(self, jmp_pos: BasicBlockId) -> Self {
fn mark_continue(self, jmp_pos: BlockNodeId) -> Self {
debug_assert!(self.0.continue_jmp.is_none());
self.0.continue_jmp = Some(jmp_pos);
self
}

fn r#break(self, bb: BasicBlockId) -> Self {
fn r#break(self, bb: BlockNodeId) -> Self {
self.0.r#break(bb);
self
}

fn r#continue(self, bb: BasicBlockId) -> Self {
fn r#continue(self, bb: BlockNodeId) -> Self {
self.0.r#continue(bb);
self
}
Expand Down
40 changes: 20 additions & 20 deletions crates/oxc_cfg/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,29 @@ mod context;

use context::Ctx;
pub use context::{CtxCursor, CtxFlags};
use oxc_index::IndexVec;
use oxc_syntax::node::NodeId;
use petgraph::Direction;

use super::{
BasicBlock, BasicBlockId, ControlFlowGraph, EdgeType, ErrorEdgeKind, Graph, Instruction,
BasicBlock, BlockNodeId, ControlFlowGraph, EdgeType, ErrorEdgeKind, Graph, Instruction,
InstructionKind, IterationInstructionKind, LabeledInstruction,
};
use crate::ReturnInstructionKind;
use crate::{BasicBlockId, ReturnInstructionKind};

#[derive(Debug, Default)]
struct ErrorHarness(ErrorEdgeKind, BasicBlockId);
struct ErrorHarness(ErrorEdgeKind, BlockNodeId);

#[derive(Debug, Default)]
pub struct ControlFlowGraphBuilder<'a> {
pub graph: Graph,
pub basic_blocks: Vec<BasicBlock>,
pub current_node_ix: BasicBlockId,
pub basic_blocks: IndexVec<BasicBlockId, BasicBlock>,
pub current_node_ix: BlockNodeId,
ctx_stack: Vec<Ctx<'a>>,
/// Contains the error unwinding path represented as a stack of `ErrorHarness`es
error_path: Vec<ErrorHarness>,
/// Stack of finalizers, the top most element is always the appropriate one for current node.
finalizers: Vec<Option<BasicBlockId>>,
finalizers: Vec<Option<BlockNodeId>>,
}

impl<'a> ControlFlowGraphBuilder<'a> {
Expand All @@ -36,7 +37,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
}

/// # Panics
pub fn basic_block(&self, basic_block: BasicBlockId) -> &BasicBlock {
pub fn basic_block(&self, basic_block: BlockNodeId) -> &BasicBlock {
let idx = *self
.graph
.node_weight(basic_block)
Expand All @@ -47,7 +48,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
}

/// # Panics
pub fn basic_block_mut(&mut self, basic_block: BasicBlockId) -> &mut BasicBlock {
pub fn basic_block_mut(&mut self, basic_block: BlockNodeId) -> &mut BasicBlock {
let idx = *self
.graph
.node_weight(basic_block)
Expand All @@ -57,23 +58,22 @@ impl<'a> ControlFlowGraphBuilder<'a> {
.expect("expected `self.current_node_ix` to be a valid node index in self.graph")
}

pub(self) fn new_basic_block(&mut self) -> BasicBlockId {
pub(self) fn new_basic_block(&mut self) -> BlockNodeId {
// current length would be the index of block we are adding on the next line.
let basic_block_ix = self.basic_blocks.len();
self.basic_blocks.push(BasicBlock::new());
let basic_block_ix = self.basic_blocks.push(BasicBlock::new());
self.graph.add_node(basic_block_ix)
}

#[must_use]
pub fn new_basic_block_function(&mut self) -> BasicBlockId {
pub fn new_basic_block_function(&mut self) -> BlockNodeId {
// we might want to differentiate between function blocks and normal blocks down the road.
self.new_basic_block_normal()
}

/// # Panics
/// if there is no error harness to attach to.
#[must_use]
pub fn new_basic_block_normal(&mut self) -> BasicBlockId {
pub fn new_basic_block_normal(&mut self) -> BlockNodeId {
let graph_ix = self.new_basic_block();
self.current_node_ix = graph_ix;

Expand All @@ -89,7 +89,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
graph_ix
}

pub fn add_edge(&mut self, a: BasicBlockId, b: BasicBlockId, weight: EdgeType) {
pub fn add_edge(&mut self, a: BlockNodeId, b: BlockNodeId, weight: EdgeType) {
if matches!(weight, EdgeType::NewFunction) {
self.basic_block_mut(b).mark_as_reachable();
} else if matches!(weight, EdgeType::Unreachable) || self.basic_block(a).is_unreachable() {
Expand Down Expand Up @@ -117,7 +117,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {

/// Creates and push a new `BasicBlockId` onto `self.error_path` stack.
/// Returns the `BasicBlockId` of the created error harness block.
pub fn attach_error_harness(&mut self, kind: ErrorEdgeKind) -> BasicBlockId {
pub fn attach_error_harness(&mut self, kind: ErrorEdgeKind) -> BlockNodeId {
let graph_ix = self.new_basic_block();
self.error_path.push(ErrorHarness(kind, graph_ix));
graph_ix
Expand All @@ -126,7 +126,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
/// # Panics
/// if there is no error harness pushed onto the stack,
/// Or last harness doesn't match the expected `BasicBlockId`.
pub fn release_error_harness(&mut self, expect: BasicBlockId) {
pub fn release_error_harness(&mut self, expect: BlockNodeId) {
let harness = self
.error_path
.pop()
Expand All @@ -139,7 +139,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {

/// Creates and push a new `BasicBlockId` onto `self.finalizers` stack.
/// Returns the `BasicBlockId` of the created finalizer block.
pub fn attach_finalizer(&mut self) -> BasicBlockId {
pub fn attach_finalizer(&mut self) -> BlockNodeId {
let graph_ix = self.new_basic_block();
self.finalizers.push(Some(graph_ix));
graph_ix
Expand All @@ -156,7 +156,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {

/// # Panics
/// if last finalizer doesn't match the expected `BasicBlockId`.
pub fn release_finalizer(&mut self, expect: BasicBlockId) {
pub fn release_finalizer(&mut self, expect: BlockNodeId) {
// return early if there is no finalizer.
let Some(finalizer) = self.finalizers.pop() else { return };
assert_eq!(
Expand All @@ -166,7 +166,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
);
}

pub fn append_condition_to(&mut self, block: BasicBlockId, node: Option<NodeId>) {
pub fn append_condition_to(&mut self, block: BlockNodeId, node: Option<NodeId>) {
self.push_instruction_to(block, InstructionKind::Condition, node);
}

Expand Down Expand Up @@ -228,7 +228,7 @@ impl<'a> ControlFlowGraphBuilder<'a> {
#[inline]
pub(self) fn push_instruction_to(
&mut self,
block: BasicBlockId,
block: BlockNodeId,
kind: InstructionKind,
node_id: Option<NodeId>,
) {
Expand Down
Loading