From 405ecdc4747d18677e9b61d0ddad04fb8f76a3e1 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Mon, 12 Jun 2023 00:00:21 +0200 Subject: [PATCH 1/5] Implement control flow graph construction --- cli/src/debug/optimizer.rs | 49 +- core/engine/src/bytecompiler/mod.rs | 8 +- .../control_flow_graph/basic_block.rs | 174 ++++++ .../src/optimizer/control_flow_graph/mod.rs | 544 ++++++++++++++++++ core/engine/src/optimizer/mod.rs | 1 + core/engine/src/vm/code_block.rs | 5 + core/engine/src/vm/mod.rs | 2 +- core/engine/src/vm/opcode/mod.rs | 44 +- 8 files changed, 818 insertions(+), 9 deletions(-) create mode 100644 core/engine/src/optimizer/control_flow_graph/basic_block.rs create mode 100644 core/engine/src/optimizer/control_flow_graph/mod.rs diff --git a/cli/src/debug/optimizer.rs b/cli/src/debug/optimizer.rs index 92793e93ba0..44bcc79d956 100644 --- a/cli/src/debug/optimizer.rs +++ b/cli/src/debug/optimizer.rs @@ -1,9 +1,15 @@ use boa_engine::{ + builtins::function::OrdinaryFunction, js_string, object::{FunctionObjectBuilder, ObjectInitializer}, - optimizer::OptimizerOptions, + optimizer::{ + control_flow_graph::{ + ControlFlowGraph, GraphEliminateUnreachableBasicBlocks, GraphSimplification, + }, + OptimizerOptions, + }, property::Attribute, - Context, JsArgs, JsObject, JsResult, JsValue, NativeFunction, + Context, JsArgs, JsNativeError, JsObject, JsResult, JsValue, NativeFunction, }; fn get_constant_folding(_: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { @@ -36,6 +42,44 @@ fn set_statistics(_: &JsValue, args: &[JsValue], context: &mut Context) -> JsRes Ok(JsValue::undefined()) } +fn graph(_: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult { + let Some(value) = args.get(0) else { + return Err(JsNativeError::typ() + .with_message("expected function argument") + .into()); + }; + + let Some(object) = value.as_object() else { + return Err(JsNativeError::typ() + .with_message(format!("expected object, got {}", value.type_of())) + .into()); + }; + let object = object.borrow(); + let Some(function) = object.downcast_ref::() else { + return Err(JsNativeError::typ() + .with_message("expected function object") + .into()); + }; + let code = function.codeblock(); + + let cfg = ControlFlowGraph::generate(code.bytecode()); + // println!("{:#?}", cfg); + + let bytecode = cfg.finalize(); + assert_eq!(code.bytecode(), &bytecode); + + let mut cfg = ControlFlowGraph::generate(&bytecode); + println!("Original\n{cfg:#?}\n"); + + let changed = GraphSimplification::perform(&mut cfg); + println!("Simplified({changed}) \n{cfg:#?}"); + + let changed = GraphEliminateUnreachableBasicBlocks::perform(&mut cfg); + println!("Eliminate Unreachble({changed}) \n{cfg:#?}"); + + Ok(JsValue::undefined()) +} + pub(super) fn create_object(context: &mut Context) -> JsObject { let get_constant_folding = FunctionObjectBuilder::new( context.realm(), @@ -75,5 +119,6 @@ pub(super) fn create_object(context: &mut Context) -> JsObject { Some(set_statistics), Attribute::WRITABLE | Attribute::CONFIGURABLE | Attribute::NON_ENUMERABLE, ) + .function(NativeFunction::from_fn_ptr(graph), js_string!("graph"), 1) .build() } diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index dc13a911d36..75fff30d8d1 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -17,6 +17,7 @@ use crate::{ builtins::function::ThisMode, environments::{BindingLocator, BindingLocatorError, CompileTimeEnvironment}, js_string, + optimizer::control_flow_graph::ControlFlowGraph, vm::{ BindingOpcode, CodeBlock, CodeBlockFlags, Constant, GeneratorResumeKind, Handler, InlineCache, Opcode, VaryingOperandKind, @@ -1521,12 +1522,17 @@ impl<'ctx> ByteCompiler<'ctx> { } self.r#return(false); + // FIXME: remove this, this is used to ensure that `finalize` works correctly. + let graph = ControlFlowGraph::generate(&self.bytecode); + let bytecode = graph.finalize().into_boxed_slice(); + assert_eq!(self.bytecode.as_slice(), bytecode.as_ref()); + CodeBlock { name: self.function_name, length: self.length, this_mode: self.this_mode, params: self.params, - bytecode: self.bytecode.into_boxed_slice(), + bytecode, constants: self.constants, bindings: self.bindings.into_boxed_slice(), handlers: self.handlers, diff --git a/core/engine/src/optimizer/control_flow_graph/basic_block.rs b/core/engine/src/optimizer/control_flow_graph/basic_block.rs new file mode 100644 index 00000000000..1eb509e0747 --- /dev/null +++ b/core/engine/src/optimizer/control_flow_graph/basic_block.rs @@ -0,0 +1,174 @@ +use std::{ + cell::RefCell, + hash::{Hash, Hasher}, + ops::Deref, + rc::{Rc, Weak}, +}; + +use bitflags::bitflags; + +use crate::vm::Instruction; + +use super::Terminator; + +bitflags! { + #[derive(Default, Clone, Copy, Debug, PartialEq, Eq, Hash)] + pub(crate) struct BasicBlockFlags: u8 { + const REACHABLE = 0b0000_0001; + } +} + +/// TODO: doc +#[derive(Default, Clone)] +pub struct BasicBlock { + pub(crate) predecessors: Vec, + pub(crate) instructions: Vec, + pub(crate) terminator: Terminator, + + pub(crate) flags: BasicBlockFlags, +} + +impl BasicBlock { + /// Get nth instruction in the [`BasicBlock`]. + pub(crate) fn get(&mut self, nth: usize) -> Option<&Instruction> { + self.instructions.get(nth) + } + + /// Insert nth instruction in the [`BasicBlock`]. + pub(crate) fn insert(&mut self, nth: usize, instruction: Instruction) { + self.instructions.insert(nth, instruction); + } + + /// Insert instruction in the last position in the [`BasicBlock`]. + pub(crate) fn insert_last(&mut self, instruction: Instruction) { + self.instructions.push(instruction); + } + + /// Remove nth instruction in the [`BasicBlock`]. + pub(crate) fn remove(&mut self, nth: usize) -> Instruction { + self.instructions.remove(nth) + } + + /// Remove last instruction in the [`BasicBlock`]. + pub(crate) fn remove_last(&mut self) -> bool { + self.instructions.pop().is_some() + } + + pub(crate) fn reachable(&self) -> bool { + self.flags.contains(BasicBlockFlags::REACHABLE) + } + + pub(crate) fn successors(&self) -> Vec { + match &self.terminator { + Terminator::None => vec![], + Terminator::JumpUnconditional { target, .. } => { + vec![target.clone()] + } + Terminator::JumpConditional { no, yes, .. } + | Terminator::TemplateLookup { no, yes, .. } => { + vec![no.clone(), yes.clone()] + } + Terminator::Return { finally } => { + let mut successors = Vec::new(); + if let Some(finally) = finally { + successors.push(finally.clone()); + } + successors + } + } + } + + pub(crate) fn next(&self, nexts: &mut Vec) { + match &self.terminator { + Terminator::None => {} + Terminator::JumpUnconditional { target, .. } => { + nexts.push(target.clone()); + } + Terminator::JumpConditional { no, yes, .. } + | Terminator::TemplateLookup { no, yes, .. } => { + nexts.push(no.clone()); + nexts.push(yes.clone()); + } + Terminator::Return { finally } => { + if let Some(_finally) = finally { + // FIXME: should we include this?? + // nexts.push(finally.clone()); + } + } + } + } +} + +/// Reference counted [`BasicBlock`] with interor mutability. +#[derive(Default, Clone)] +pub struct RcBasicBlock { + inner: Rc>, +} + +impl From>> for RcBasicBlock { + fn from(inner: Rc>) -> Self { + Self { inner } + } +} + +impl Deref for RcBasicBlock { + type Target = Rc>; + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl PartialEq for RcBasicBlock { + fn eq(&self, other: &RcBasicBlock) -> bool { + Rc::ptr_eq(&self.inner, &other.inner) + } +} + +impl Eq for RcBasicBlock {} + +impl Hash for RcBasicBlock { + fn hash(&self, state: &mut H) { + (self.as_ptr() as usize).hash(state); + } +} + +impl RcBasicBlock { + /// TODO: doc + #[must_use] + pub fn downgrade(&self) -> WeakBasicBlock { + WeakBasicBlock::from(Rc::downgrade(&self.inner)) + } +} + +/// Reference counted [`BasicBlock`] with interor mutability. +#[derive(Default, Clone)] +pub struct WeakBasicBlock { + inner: Weak>, +} + +impl From>> for WeakBasicBlock { + fn from(inner: Weak>) -> Self { + Self { inner } + } +} + +impl Deref for WeakBasicBlock { + type Target = Weak>; + fn deref(&self) -> &Self::Target { + &self.inner + } +} + +impl PartialEq for WeakBasicBlock { + fn eq(&self, other: &WeakBasicBlock) -> bool { + Weak::ptr_eq(&self.inner, &other.inner) + } +} + +impl WeakBasicBlock { + /// TODO: doc + #[must_use] + pub fn upgrade(&self) -> Option { + Some(RcBasicBlock::from(self.inner.upgrade()?)) + } +} diff --git a/core/engine/src/optimizer/control_flow_graph/mod.rs b/core/engine/src/optimizer/control_flow_graph/mod.rs new file mode 100644 index 00000000000..ef8ff3f8e02 --- /dev/null +++ b/core/engine/src/optimizer/control_flow_graph/mod.rs @@ -0,0 +1,544 @@ +//! TODO: doc + +#![allow(dead_code)] +#![allow(missing_debug_implementations)] + +mod basic_block; + +use std::{fmt::Debug, rc::Rc}; + +use indexmap::IndexSet; +use rustc_hash::FxHashMap; + +use crate::vm::{Instruction, InstructionIterator, Opcode}; + +use self::basic_block::BasicBlockFlags; +pub use self::basic_block::{BasicBlock, RcBasicBlock, WeakBasicBlock}; + +/// TODO: doc +#[derive(Default, Clone)] +pub enum Terminator { + /// TODO: doc + #[default] + None, + + /// TODO: doc + JumpUnconditional { + /// TODO: doc + opcode: Opcode, + /// TODO: doc + target: RcBasicBlock, + }, + + /// TODO: doc + JumpConditional { + /// TODO: doc + opcode: Opcode, + /// TODO: doc + no: RcBasicBlock, + /// TODO: doc + yes: RcBasicBlock, + }, + + /// TODO: doc + TemplateLookup { + /// TODO: doc + no: RcBasicBlock, + + /// TODO: doc + yes: RcBasicBlock, + + /// TODO: doc + site: u64, + }, + + /// TODO: doc + Return { + /// Finally block that the return should jump to, if exists. + finally: Option, + }, +} + +impl Terminator { + /// Check if [`Terminator::None`]. + #[must_use] + pub fn is_none(&self) -> bool { + matches!(self, Terminator::None) + } + + /// Check if [`Terminator::Jump`]. + #[must_use] + pub fn is_jump(&self) -> bool { + matches!( + self, + Terminator::JumpUnconditional { .. } | Terminator::JumpConditional { .. } + ) + } + + /// Check if unconditional [`Terminator::Jump`]. + #[must_use] + pub fn is_unconditional_jump(&self) -> bool { + matches!(self, Terminator::JumpUnconditional { .. }) + } + + /// Check if conditional [`Terminator::Jump`]. + #[must_use] + pub fn is_conditional_jump(&self) -> bool { + matches!(self, Terminator::JumpConditional { .. }) + } +} + +/// TODO: doc +pub struct ControlFlowGraph { + basic_block_start: RcBasicBlock, + basic_blocks: IndexSet, +} + +impl Debug for ControlFlowGraph { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + writeln!(f, "BasicBlocks:")?; + + let mut seen = FxHashMap::default(); + let index_from_basic_block = |bb: &RcBasicBlock| { + for (i, basic_block) in self.basic_blocks.iter().enumerate() { + if basic_block == bb { + return i; + } + } + + unreachable!("There should be a basic block") + }; + + let mut index = 0; + for basic_block in &self.basic_blocks { + if seen.contains_key(&basic_block.as_ptr()) { + continue; + } + seen.insert(basic_block.as_ptr(), index); + + let basic_block = basic_block.borrow(); + + write!( + f, + " B{index}: -- {}reachable", + if basic_block.reachable() { "" } else { "not " } + )?; + + if !basic_block.predecessors.is_empty() { + write!(f, " -- predecessors ")?; + for predecessor in &basic_block.predecessors { + if let Some(predecessor) = predecessor.upgrade() { + let index = index_from_basic_block(&predecessor); + write!(f, "B{index}, ")?; + } + } + } + + let successors = basic_block.successors(); + if !successors.is_empty() { + write!(f, " -- successors ")?; + for successor in &successors { + let index = index_from_basic_block(successor); + write!(f, "B{index}, ")?; + } + } + + writeln!(f)?; + + for (i, result) in basic_block.instructions.iter().enumerate() { + writeln!(f, " {:06} {}", i, result.opcode().as_str())?; + } + + let terminator = &basic_block.terminator; + if !terminator.is_none() { + write!(f, " Terminator: ")?; + match terminator { + Terminator::None => write!(f, "None")?, + Terminator::JumpUnconditional { opcode, target } => { + let target = index_from_basic_block(target); + write!(f, "{} B{target}", opcode.as_str())?; + } + Terminator::JumpConditional { opcode, no: _, yes } => { + let target = index_from_basic_block(yes); + write!(f, "{} B{target}", opcode.as_str())?; + } + Terminator::TemplateLookup { + no: _, + yes, + site: _, + } => { + let target = index_from_basic_block(yes); + write!(f, "TemplateLookup B{target}")?; + } + Terminator::Return { finally } => { + write!(f, "Return")?; + if let Some(finally) = finally { + let finally = index_from_basic_block(finally); + write!(f, " -- finally block B{finally}")?; + } + } + } + writeln!(f)?; + } + + writeln!(f)?; + + index += 1; + } + + Ok(()) + } +} + +const fn is_jump_kind_instruction(instruction: &Instruction) -> Option { + match instruction { + Instruction::Jump { address } + | Instruction::JumpIfTrue { address } + | Instruction::JumpIfFalse { address } + | Instruction::JumpIfNotUndefined { address } + | Instruction::JumpIfNullOrUndefined { address } + | Instruction::Case { address } + | Instruction::Default { address } + | Instruction::LogicalAnd { exit: address } + | Instruction::LogicalOr { exit: address } + | Instruction::Coalesce { exit: address } => Some(*address), + _ => None, + } +} + +impl ControlFlowGraph { + /// Generate leaders for the [`BasicBlock`]s. + fn leaders(bytecode: &[u8]) -> Vec { + let mut leaders: Vec = vec![]; + + let mut iter = InstructionIterator::new(bytecode); + + while let Some((pc, _, instruction)) = iter.next() { + // println!("{pc:4} {instruction:?}"); + match instruction { + Instruction::Return => { + leaders.push(iter.pc() as u32); + } + Instruction::TemplateLookup { exit, .. } => { + leaders.push(iter.pc() as u32); + leaders.push(exit); + } + instruction => { + if let Some(target) = is_jump_kind_instruction(&instruction) { + leaders.push(iter.pc() as u32); + leaders.push(target); + } + } + } + } + + leaders.push(0); + leaders.sort_unstable(); + leaders.dedup(); + + // println!("leaders: {leaders:?}"); + + leaders + } + + /// TODO: doc + #[must_use] + pub fn generate(bytecode: &[u8]) -> Self { + let leaders = Self::leaders(bytecode); + let block_count = leaders.len(); + + let mut basic_blocks = IndexSet::with_capacity(block_count); + for _ in 0..block_count { + basic_blocks.insert(RcBasicBlock::default()); + } + + let basic_block_from_bytecode_position = |address: u32| { + let index = leaders + .iter() + .position(|x| *x == address) + .expect("There should be a basic block"); + + basic_blocks[index].clone() + }; + + let mut iter = InstructionIterator::new(bytecode); + for (i, leader) in leaders + .iter() + .map(|x| *x as usize) + .enumerate() + .skip(1) + .map(|(i, leader)| (i - 1, leader)) + { + let this = basic_blocks[i].clone(); + + let mut bytecode = Vec::new(); + let mut terminator = Terminator::None; + while let Some((_, _, instruction)) = iter.next() { + match instruction { + Instruction::Return => { + terminator = Terminator::Return { finally: None }; + } + Instruction::Jump { address } | Instruction::Default { address } => { + let target = basic_block_from_bytecode_position(address); + + target.borrow_mut().predecessors.push(this.downgrade()); + + terminator = Terminator::JumpUnconditional { + opcode: instruction.opcode(), + target, + }; + } + Instruction::TemplateLookup { + exit: address, + site, + } => { + let yes = basic_block_from_bytecode_position(address); + let no = basic_blocks[i + 1].clone(); + + yes.borrow_mut().predecessors.push(this.downgrade()); + no.borrow_mut().predecessors.push(this.downgrade()); + + terminator = Terminator::TemplateLookup { no, yes, site }; + } + instruction => { + if let Some(address) = is_jump_kind_instruction(&instruction) { + let yes = basic_block_from_bytecode_position(address); + let no = basic_blocks[i + 1].clone(); + + yes.borrow_mut().predecessors.push(this.downgrade()); + no.borrow_mut().predecessors.push(this.downgrade()); + + terminator = Terminator::JumpConditional { + opcode: instruction.opcode(), + no, + yes, + }; + } else { + bytecode.push(instruction); + } + } + } + + if leader <= iter.pc() { + break; + } + } + + let mut basic_block = this.borrow_mut(); + basic_block.instructions = bytecode; + basic_block.terminator = terminator; + } + + Self { + basic_block_start: basic_blocks[0].clone(), + basic_blocks, + } + } + + /// Remove [`BasicBlock`]. + pub fn remove(&mut self, basic_block: &RcBasicBlock) { + self.basic_blocks.shift_remove(basic_block); + } + + /// Get [`BasicBlock`] index. + #[must_use] + pub fn get_index(&self, basic_block: &RcBasicBlock) -> usize { + self.basic_blocks + .get_index_of(basic_block) + .expect("there should be a BasicBlock in CFG") + } + + /// Finalize bytecode. + #[must_use] + pub fn finalize(self) -> Vec { + let index_from_basic_block = |bb: &RcBasicBlock| { + for (i, basic_block) in self.basic_blocks.iter().enumerate() { + if Rc::ptr_eq(basic_block, bb) { + return i; + } + } + + unreachable!("There should be a basic block") + }; + + let mut results = Vec::new(); + let mut labels = Vec::new(); + let mut blocks = Vec::with_capacity(self.basic_blocks.len()); + + for basic_block in &self.basic_blocks { + let basic_block = basic_block.borrow(); + + blocks.push(results.len() as u32); + + for instruction in &basic_block.instructions { + instruction.to_bytecode(&mut results); + } + + match &basic_block.terminator { + Terminator::None => {} + Terminator::JumpUnconditional { opcode, target } => { + results.extend_from_slice(&[*opcode as u8]); + let start = results.len(); + results.extend_from_slice(&[0, 0, 0, 0]); + + let target = index_from_basic_block(target); + labels.push((start as u32, target)); + } + Terminator::JumpConditional { opcode, no: _, yes } => { + results.extend_from_slice(&[*opcode as u8]); + let start = results.len(); + results.extend_from_slice(&[0, 0, 0, 0]); + + let target = index_from_basic_block(yes); + labels.push((start as u32, target)); + } + Terminator::TemplateLookup { yes, site, .. } => { + results.extend_from_slice(&[Opcode::TemplateLookup as u8]); + let start = results.len(); + results.extend_from_slice(&[0, 0, 0, 0]); + results.extend_from_slice(&site.to_ne_bytes()); + + let target = index_from_basic_block(yes); + labels.push((start as u32, target)); + } + Terminator::Return { .. } => { + results.push(Opcode::Return as u8); + } + } + } + + for (label, block_index) in labels { + let address = blocks[block_index]; + + let bytes = address.to_ne_bytes(); + results[label as usize] = bytes[0]; + results[label as usize + 1] = bytes[1]; + results[label as usize + 2] = bytes[2]; + results[label as usize + 3] = bytes[3]; + } + + results + } +} + +impl Drop for ControlFlowGraph { + fn drop(&mut self) { + // NOTE: Untie BasicBlock nodes, so they can be deallocated. + for basic_block in &self.basic_blocks { + *basic_block.borrow_mut() = BasicBlock::default(); + } + } +} + +/// Simplifies the [`ControlFlowGraph`]. +/// +/// # Operations +/// +/// - Branch to same blocks -> jump +/// - Unrachable block elimination +#[derive(Clone, Copy)] +pub struct GraphSimplification; + +impl GraphSimplification { + /// TODO: doc + pub fn perform(graph: &mut ControlFlowGraph) -> bool { + let mut changed = false; + for basic_block_ptr in &graph.basic_blocks { + { + let mut basic_block = basic_block_ptr.borrow_mut(); + + #[allow(clippy::single_match)] + match basic_block.terminator.clone() { + Terminator::JumpConditional { no, yes, .. } => { + if no == yes { + basic_block.insert_last(Instruction::Pop); + basic_block.terminator = Terminator::JumpUnconditional { + opcode: Opcode::Jump, + target: yes, + }; + + changed |= true; + } + } + _ => {} + } + } + } + changed + } +} + +/// TODO: doc +#[derive(Clone, Copy)] +pub struct GraphEliminateUnreachableBasicBlocks; + +impl GraphEliminateUnreachableBasicBlocks { + /// TODO: doc + pub fn perform(graph: &mut ControlFlowGraph) -> bool { + let mut changed = false; + + let mut stack = vec![graph.basic_block_start.clone()]; + while let Some(basic_block_ptr) = stack.pop() { + let mut basic_block = basic_block_ptr.borrow_mut(); + if basic_block.reachable() { + break; + } + basic_block.flags |= BasicBlockFlags::REACHABLE; + basic_block.next(&mut stack); + + // println!("{:p} -- {}", basic_block_ptr.as_ptr(), basic_block.reachable()); + } + + assert!( + graph.basic_block_start.borrow().reachable(), + "start basic block node should always be reachable" + ); + + let mut delete_list = Vec::new(); + for (i, basic_block) in graph.basic_blocks.iter().enumerate().rev() { + if !basic_block.borrow().reachable() { + delete_list.push(i); + } + } + + // println!("{delete_list:?}"); + + for i in delete_list { + let basic_block = graph + .basic_blocks + .shift_remove_index(i) + .expect("there should be a BasicBlock in CFG"); + let mut basic_block = basic_block.borrow_mut(); + + assert!( + !basic_block.reachable(), + "reachable basic blocks should not be eliminated" + ); + + basic_block.predecessors.clear(); + basic_block.terminator = Terminator::None; + + changed |= true; + } + + changed + } +} + +#[cfg(test)] +mod test { + use super::ControlFlowGraph; + + #[test] + fn preserve_jump() { + let bytecode = &[ + 156, 6, 120, 15, 0, 0, 0, 153, 0, 155, 118, 0, 0, 0, 0, 147, 148, + ]; + + let graph = ControlFlowGraph::generate(bytecode); + + let actual = graph.finalize(); + + assert_eq!(bytecode, actual.as_slice()); + } +} diff --git a/core/engine/src/optimizer/mod.rs b/core/engine/src/optimizer/mod.rs index 77a2716e782..1a45ba3efeb 100644 --- a/core/engine/src/optimizer/mod.rs +++ b/core/engine/src/optimizer/mod.rs @@ -1,5 +1,6 @@ //! Implements optimizations. +pub mod control_flow_graph; pub(crate) mod pass; pub(crate) mod walker; diff --git a/core/engine/src/vm/code_block.rs b/core/engine/src/vm/code_block.rs index 1ceec0f8ae7..eec4231f721 100644 --- a/core/engine/src/vm/code_block.rs +++ b/core/engine/src/vm/code_block.rs @@ -356,6 +356,11 @@ impl CodeBlock { /// ---- `CodeBlock` private API ---- impl CodeBlock { + /// Get [`CodeBlock`] bytecode. + pub fn bytecode(&self) -> &[u8] { + &self.bytecode + } + /// Read type T from code. /// /// # Safety diff --git a/core/engine/src/vm/mod.rs b/core/engine/src/vm/mod.rs index 9c54f049051..cece11dd8e4 100644 --- a/core/engine/src/vm/mod.rs +++ b/core/engine/src/vm/mod.rs @@ -17,7 +17,7 @@ use std::{future::Future, mem::size_of, ops::ControlFlow, pin::Pin, task}; use crate::sys::time::Instant; mod call_frame; -mod code_block; +pub(crate) mod code_block; mod completion_record; mod opcode; diff --git a/core/engine/src/vm/opcode/mod.rs b/core/engine/src/vm/opcode/mod.rs index 2d0bc52e797..330561c23fd 100644 --- a/core/engine/src/vm/opcode/mod.rs +++ b/core/engine/src/vm/opcode/mod.rs @@ -128,12 +128,12 @@ where } /// Represents a varying operand kind. -#[derive(Default, Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub(crate) enum VaryingOperandKind { #[default] - U8, - U16, - U32, + U8 = 0, + U16 = 1, + U32 = 2, } #[derive(Debug, Clone, Copy)] @@ -354,6 +354,31 @@ impl BytecodeConversion for ThinVec { } } +trait VaryingKindTrait { + fn get_varying_kind(&self) -> VaryingOperandKind { + VaryingOperandKind::U8 + } +} + +impl VaryingKindTrait for u8 {} +impl VaryingKindTrait for i8 {} +impl VaryingKindTrait for u16 {} +impl VaryingKindTrait for i16 {} +impl VaryingKindTrait for u32 {} +impl VaryingKindTrait for i32 {} +impl VaryingKindTrait for u64 {} +impl VaryingKindTrait for i64 {} +impl VaryingKindTrait for f32 {} +impl VaryingKindTrait for f64 {} +impl VaryingKindTrait for bool {} +impl VaryingKindTrait for GeneratorResumeKind {} +impl VaryingKindTrait for ThinVec {} +impl VaryingKindTrait for VaryingOperand { + fn get_varying_kind(&self) -> VaryingOperandKind { + self.kind() + } +} + /// Generate [`Opcode`]s and [`Instruction`]s enums. macro_rules! generate_opcodes { ( name $name:ident ) => { $name }; @@ -484,13 +509,22 @@ macro_rules! generate_opcodes { impl Instruction { /// Convert [`Instruction`] to compact bytecode. #[inline] - #[allow(dead_code)] + #[allow(unused_mut)] pub(crate) fn to_bytecode(&self, bytes: &mut Vec) { match self { $( Self::$Variant $({ $( $FieldName ),* })? => { + let mut kind = VaryingOperandKind::U8; + $({ + $( kind = VaryingKindTrait::get_varying_kind($FieldName).max(kind); )* + })? + match kind { + VaryingOperandKind::U8 => {}, + VaryingOperandKind::U16 => bytes.push(Opcode::U16Operands as u8), + VaryingOperandKind::U32 => bytes.push(Opcode::U32Operands as u8), + } bytes.push(Opcode::$Variant as u8); $({ $( BytecodeConversion::to_bytecode($FieldName, bytes); )* From b7b62939165a05db07fbd339bf591b157f873788 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sat, 9 Dec 2023 15:15:03 +0100 Subject: [PATCH 2/5] Add handlers --- cli/src/debug/optimizer.rs | 16 +++--- core/engine/src/bytecompiler/mod.rs | 2 +- .../control_flow_graph/basic_block.rs | 17 +----- .../src/optimizer/control_flow_graph/mod.rs | 55 +++++++++++++------ 4 files changed, 49 insertions(+), 41 deletions(-) diff --git a/cli/src/debug/optimizer.rs b/cli/src/debug/optimizer.rs index 44bcc79d956..98fddc706b1 100644 --- a/cli/src/debug/optimizer.rs +++ b/cli/src/debug/optimizer.rs @@ -62,20 +62,20 @@ fn graph(_: &JsValue, args: &[JsValue], _context: &mut Context) -> JsResult ByteCompiler<'ctx> { self.r#return(false); // FIXME: remove this, this is used to ensure that `finalize` works correctly. - let graph = ControlFlowGraph::generate(&self.bytecode); + let graph = ControlFlowGraph::generate(&self.bytecode, &self.handlers); let bytecode = graph.finalize().into_boxed_slice(); assert_eq!(self.bytecode.as_slice(), bytecode.as_ref()); diff --git a/core/engine/src/optimizer/control_flow_graph/basic_block.rs b/core/engine/src/optimizer/control_flow_graph/basic_block.rs index 1eb509e0747..d1c9bfc5167 100644 --- a/core/engine/src/optimizer/control_flow_graph/basic_block.rs +++ b/core/engine/src/optimizer/control_flow_graph/basic_block.rs @@ -24,6 +24,7 @@ pub struct BasicBlock { pub(crate) predecessors: Vec, pub(crate) instructions: Vec, pub(crate) terminator: Terminator, + pub(crate) handler: Option, pub(crate) flags: BasicBlockFlags, } @@ -68,19 +69,13 @@ impl BasicBlock { | Terminator::TemplateLookup { no, yes, .. } => { vec![no.clone(), yes.clone()] } - Terminator::Return { finally } => { - let mut successors = Vec::new(); - if let Some(finally) = finally { - successors.push(finally.clone()); - } - successors - } + Terminator::Return => Vec::new(), } } pub(crate) fn next(&self, nexts: &mut Vec) { match &self.terminator { - Terminator::None => {} + Terminator::None | Terminator::Return => {} Terminator::JumpUnconditional { target, .. } => { nexts.push(target.clone()); } @@ -89,12 +84,6 @@ impl BasicBlock { nexts.push(no.clone()); nexts.push(yes.clone()); } - Terminator::Return { finally } => { - if let Some(_finally) = finally { - // FIXME: should we include this?? - // nexts.push(finally.clone()); - } - } } } } diff --git a/core/engine/src/optimizer/control_flow_graph/mod.rs b/core/engine/src/optimizer/control_flow_graph/mod.rs index ef8ff3f8e02..b5f1c3a19e5 100644 --- a/core/engine/src/optimizer/control_flow_graph/mod.rs +++ b/core/engine/src/optimizer/control_flow_graph/mod.rs @@ -10,7 +10,7 @@ use std::{fmt::Debug, rc::Rc}; use indexmap::IndexSet; use rustc_hash::FxHashMap; -use crate::vm::{Instruction, InstructionIterator, Opcode}; +use crate::vm::{CodeBlock, Handler, Instruction, InstructionIterator, Opcode}; use self::basic_block::BasicBlockFlags; pub use self::basic_block::{BasicBlock, RcBasicBlock, WeakBasicBlock}; @@ -53,10 +53,7 @@ pub enum Terminator { }, /// TODO: doc - Return { - /// Finally block that the return should jump to, if exists. - finally: Option, - }, + Return, } impl Terminator { @@ -143,6 +140,11 @@ impl Debug for ControlFlowGraph { } } + if let Some(handler) = &basic_block.handler { + let index = index_from_basic_block(handler); + write!(f, " -- handler B{index}")?; + } + writeln!(f)?; for (i, result) in basic_block.instructions.iter().enumerate() { @@ -170,12 +172,8 @@ impl Debug for ControlFlowGraph { let target = index_from_basic_block(yes); write!(f, "TemplateLookup B{target}")?; } - Terminator::Return { finally } => { + Terminator::Return => { write!(f, "Return")?; - if let Some(finally) = finally { - let finally = index_from_basic_block(finally); - write!(f, " -- finally block B{finally}")?; - } } } writeln!(f)?; @@ -208,12 +206,17 @@ const fn is_jump_kind_instruction(instruction: &Instruction) -> Option { impl ControlFlowGraph { /// Generate leaders for the [`BasicBlock`]s. - fn leaders(bytecode: &[u8]) -> Vec { - let mut leaders: Vec = vec![]; + fn leaders(bytecode: &[u8], handlers: &[Handler]) -> Vec { + let mut leaders = Vec::new(); let mut iter = InstructionIterator::new(bytecode); - while let Some((pc, _, instruction)) = iter.next() { + for handler in handlers { + leaders.push(handler.start); + leaders.push(handler.handler()); + } + + while let Some((_, _, instruction)) = iter.next() { // println!("{pc:4} {instruction:?}"); match instruction { Instruction::Return => { @@ -243,8 +246,14 @@ impl ControlFlowGraph { /// TODO: doc #[must_use] - pub fn generate(bytecode: &[u8]) -> Self { - let leaders = Self::leaders(bytecode); + pub fn generate_from_codeblock(code: &CodeBlock) -> Self { + Self::generate(&code.bytecode, &code.handlers) + } + + /// TODO: doc + #[must_use] + pub fn generate(bytecode: &[u8], handlers: &[Handler]) -> Self { + let leaders = Self::leaders(bytecode, handlers); let block_count = leaders.len(); let mut basic_blocks = IndexSet::with_capacity(block_count); @@ -271,12 +280,22 @@ impl ControlFlowGraph { { let this = basic_blocks[i].clone(); + let handler = handlers + .iter() + .rev() + .find(|handler| handler.contains(iter.pc() as u32)); + if let Some(handler) = handler { + let handler = basic_block_from_bytecode_position(handler.handler()); + + this.borrow_mut().handler = Some(handler); + } + let mut bytecode = Vec::new(); let mut terminator = Terminator::None; while let Some((_, _, instruction)) = iter.next() { match instruction { Instruction::Return => { - terminator = Terminator::Return { finally: None }; + terminator = Terminator::Return; } Instruction::Jump { address } | Instruction::Default { address } => { let target = basic_block_from_bytecode_position(address); @@ -434,7 +453,7 @@ impl Drop for ControlFlowGraph { /// /// # Operations /// -/// - Branch to same blocks -> jump +/// - Conditional Branch to same blocks -> unconditional /// - Unrachable block elimination #[derive(Clone, Copy)] pub struct GraphSimplification; @@ -535,7 +554,7 @@ mod test { 156, 6, 120, 15, 0, 0, 0, 153, 0, 155, 118, 0, 0, 0, 0, 147, 148, ]; - let graph = ControlFlowGraph::generate(bytecode); + let graph = ControlFlowGraph::generate(bytecode, &[]); let actual = graph.finalize(); From bc2318294a4eeaddd3c5c1644fbf2db9a2ede3fa Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sat, 9 Dec 2023 16:25:53 +0100 Subject: [PATCH 3/5] Use `slotmap` instead of `Rc` for `BasicBlock`s --- Cargo.lock | 10 + cli/src/debug/optimizer.rs | 7 +- core/engine/Cargo.toml | 1 + .../control_flow_graph/basic_block.rs | 105 +------ .../src/optimizer/control_flow_graph/mod.rs | 281 +++++++++--------- 5 files changed, 159 insertions(+), 245 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 76e74165cd1..feb54f30860 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -472,6 +472,7 @@ dependencies = [ "ryu-js", "serde", "serde_json", + "slotmap", "sptr", "static_assertions", "sys-locale", @@ -3407,6 +3408,15 @@ dependencies = [ "autocfg", ] +[[package]] +name = "slotmap" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbff4acf519f630b3a3ddcfaea6c06b42174d9a44bc70c620e9ed1649d58b82a" +dependencies = [ + "version_check", +] + [[package]] name = "smallvec" version = "1.11.2" diff --git a/cli/src/debug/optimizer.rs b/cli/src/debug/optimizer.rs index 98fddc706b1..e43ebdfd169 100644 --- a/cli/src/debug/optimizer.rs +++ b/cli/src/debug/optimizer.rs @@ -2,12 +2,7 @@ use boa_engine::{ builtins::function::OrdinaryFunction, js_string, object::{FunctionObjectBuilder, ObjectInitializer}, - optimizer::{ - control_flow_graph::{ - ControlFlowGraph, GraphEliminateUnreachableBasicBlocks, GraphSimplification, - }, - OptimizerOptions, - }, + optimizer::{control_flow_graph::ControlFlowGraph, OptimizerOptions}, property::Attribute, Context, JsArgs, JsNativeError, JsObject, JsResult, JsValue, NativeFunction, }; diff --git a/core/engine/Cargo.toml b/core/engine/Cargo.toml index b4ca2504d80..a7306392a30 100644 --- a/core/engine/Cargo.toml +++ b/core/engine/Cargo.toml @@ -93,6 +93,7 @@ bytemuck = { version = "1.14.0", features = ["derive"] } arrayvec = "0.7.4" intrusive-collections = "0.9.6" cfg-if = "1.0.0" +slotmap = { version = "1.0", default-features = false } # intl deps boa_icu_provider = {workspace = true, features = ["std"], optional = true } diff --git a/core/engine/src/optimizer/control_flow_graph/basic_block.rs b/core/engine/src/optimizer/control_flow_graph/basic_block.rs index d1c9bfc5167..f66fb33dc39 100644 --- a/core/engine/src/optimizer/control_flow_graph/basic_block.rs +++ b/core/engine/src/optimizer/control_flow_graph/basic_block.rs @@ -1,15 +1,10 @@ -use std::{ - cell::RefCell, - hash::{Hash, Hasher}, - ops::Deref, - rc::{Rc, Weak}, -}; +use std::hash::Hash; use bitflags::bitflags; use crate::vm::Instruction; -use super::Terminator; +use super::{BasicBlockKey, Terminator}; bitflags! { #[derive(Default, Clone, Copy, Debug, PartialEq, Eq, Hash)] @@ -21,10 +16,10 @@ bitflags! { /// TODO: doc #[derive(Default, Clone)] pub struct BasicBlock { - pub(crate) predecessors: Vec, + pub(crate) predecessors: Vec, pub(crate) instructions: Vec, pub(crate) terminator: Terminator, - pub(crate) handler: Option, + pub(crate) handler: Option, pub(crate) flags: BasicBlockFlags, } @@ -59,105 +54,31 @@ impl BasicBlock { self.flags.contains(BasicBlockFlags::REACHABLE) } - pub(crate) fn successors(&self) -> Vec { - match &self.terminator { + pub(crate) fn successors(&self) -> Vec { + match self.terminator { Terminator::None => vec![], Terminator::JumpUnconditional { target, .. } => { - vec![target.clone()] + vec![target] } Terminator::JumpConditional { no, yes, .. } | Terminator::TemplateLookup { no, yes, .. } => { - vec![no.clone(), yes.clone()] + vec![no, yes] } Terminator::Return => Vec::new(), } } - pub(crate) fn next(&self, nexts: &mut Vec) { - match &self.terminator { + pub(crate) fn next(&self, nexts: &mut Vec) { + match self.terminator { Terminator::None | Terminator::Return => {} Terminator::JumpUnconditional { target, .. } => { - nexts.push(target.clone()); + nexts.push(target); } Terminator::JumpConditional { no, yes, .. } | Terminator::TemplateLookup { no, yes, .. } => { - nexts.push(no.clone()); - nexts.push(yes.clone()); + nexts.push(no); + nexts.push(yes); } } } } - -/// Reference counted [`BasicBlock`] with interor mutability. -#[derive(Default, Clone)] -pub struct RcBasicBlock { - inner: Rc>, -} - -impl From>> for RcBasicBlock { - fn from(inner: Rc>) -> Self { - Self { inner } - } -} - -impl Deref for RcBasicBlock { - type Target = Rc>; - fn deref(&self) -> &Self::Target { - &self.inner - } -} - -impl PartialEq for RcBasicBlock { - fn eq(&self, other: &RcBasicBlock) -> bool { - Rc::ptr_eq(&self.inner, &other.inner) - } -} - -impl Eq for RcBasicBlock {} - -impl Hash for RcBasicBlock { - fn hash(&self, state: &mut H) { - (self.as_ptr() as usize).hash(state); - } -} - -impl RcBasicBlock { - /// TODO: doc - #[must_use] - pub fn downgrade(&self) -> WeakBasicBlock { - WeakBasicBlock::from(Rc::downgrade(&self.inner)) - } -} - -/// Reference counted [`BasicBlock`] with interor mutability. -#[derive(Default, Clone)] -pub struct WeakBasicBlock { - inner: Weak>, -} - -impl From>> for WeakBasicBlock { - fn from(inner: Weak>) -> Self { - Self { inner } - } -} - -impl Deref for WeakBasicBlock { - type Target = Weak>; - fn deref(&self) -> &Self::Target { - &self.inner - } -} - -impl PartialEq for WeakBasicBlock { - fn eq(&self, other: &WeakBasicBlock) -> bool { - Weak::ptr_eq(&self.inner, &other.inner) - } -} - -impl WeakBasicBlock { - /// TODO: doc - #[must_use] - pub fn upgrade(&self) -> Option { - Some(RcBasicBlock::from(self.inner.upgrade()?)) - } -} diff --git a/core/engine/src/optimizer/control_flow_graph/mod.rs b/core/engine/src/optimizer/control_flow_graph/mod.rs index b5f1c3a19e5..2162dff3bdd 100644 --- a/core/engine/src/optimizer/control_flow_graph/mod.rs +++ b/core/engine/src/optimizer/control_flow_graph/mod.rs @@ -5,19 +5,20 @@ mod basic_block; -use std::{fmt::Debug, rc::Rc}; +use std::fmt::Debug; -use indexmap::IndexSet; use rustc_hash::FxHashMap; +use slotmap::{new_key_type, SlotMap}; use crate::vm::{CodeBlock, Handler, Instruction, InstructionIterator, Opcode}; -use self::basic_block::BasicBlockFlags; -pub use self::basic_block::{BasicBlock, RcBasicBlock, WeakBasicBlock}; +pub use self::basic_block::BasicBlock; + +new_key_type! { pub(crate) struct BasicBlockKey; } /// TODO: doc -#[derive(Default, Clone)] -pub enum Terminator { +#[derive(Default, Clone, Copy)] +pub(crate) enum Terminator { /// TODO: doc #[default] None, @@ -27,7 +28,7 @@ pub enum Terminator { /// TODO: doc opcode: Opcode, /// TODO: doc - target: RcBasicBlock, + target: BasicBlockKey, }, /// TODO: doc @@ -35,18 +36,18 @@ pub enum Terminator { /// TODO: doc opcode: Opcode, /// TODO: doc - no: RcBasicBlock, + no: BasicBlockKey, /// TODO: doc - yes: RcBasicBlock, + yes: BasicBlockKey, }, /// TODO: doc TemplateLookup { /// TODO: doc - no: RcBasicBlock, + no: BasicBlockKey, /// TODO: doc - yes: RcBasicBlock, + yes: BasicBlockKey, /// TODO: doc site: u64, @@ -59,13 +60,13 @@ pub enum Terminator { impl Terminator { /// Check if [`Terminator::None`]. #[must_use] - pub fn is_none(&self) -> bool { + pub(crate) fn is_none(&self) -> bool { matches!(self, Terminator::None) } /// Check if [`Terminator::Jump`]. #[must_use] - pub fn is_jump(&self) -> bool { + pub(crate) fn is_jump(&self) -> bool { matches!( self, Terminator::JumpUnconditional { .. } | Terminator::JumpConditional { .. } @@ -74,21 +75,21 @@ impl Terminator { /// Check if unconditional [`Terminator::Jump`]. #[must_use] - pub fn is_unconditional_jump(&self) -> bool { + pub(crate) fn is_unconditional_jump(&self) -> bool { matches!(self, Terminator::JumpUnconditional { .. }) } /// Check if conditional [`Terminator::Jump`]. #[must_use] - pub fn is_conditional_jump(&self) -> bool { + pub(crate) fn is_conditional_jump(&self) -> bool { matches!(self, Terminator::JumpConditional { .. }) } } /// TODO: doc pub struct ControlFlowGraph { - basic_block_start: RcBasicBlock, - basic_blocks: IndexSet, + basic_block_start: BasicBlockKey, + basic_blocks: SlotMap, } impl Debug for ControlFlowGraph { @@ -96,9 +97,9 @@ impl Debug for ControlFlowGraph { writeln!(f, "BasicBlocks:")?; let mut seen = FxHashMap::default(); - let index_from_basic_block = |bb: &RcBasicBlock| { - for (i, basic_block) in self.basic_blocks.iter().enumerate() { - if basic_block == bb { + let index_from_basic_block = |bb: BasicBlockKey| { + for (i, (key, _basic_block)) in self.basic_blocks.iter().enumerate() { + if key == bb { return i; } } @@ -107,13 +108,13 @@ impl Debug for ControlFlowGraph { }; let mut index = 0; - for basic_block in &self.basic_blocks { - if seen.contains_key(&basic_block.as_ptr()) { + for key in self.basic_blocks.keys() { + if seen.contains_key(&key) { continue; } - seen.insert(basic_block.as_ptr(), index); + seen.insert(key, index); - let basic_block = basic_block.borrow(); + let basic_block = &self.basic_blocks[key]; write!( f, @@ -124,10 +125,8 @@ impl Debug for ControlFlowGraph { if !basic_block.predecessors.is_empty() { write!(f, " -- predecessors ")?; for predecessor in &basic_block.predecessors { - if let Some(predecessor) = predecessor.upgrade() { - let index = index_from_basic_block(&predecessor); - write!(f, "B{index}, ")?; - } + let index = index_from_basic_block(*predecessor); + write!(f, "B{index}, ")?; } } @@ -135,13 +134,13 @@ impl Debug for ControlFlowGraph { if !successors.is_empty() { write!(f, " -- successors ")?; for successor in &successors { - let index = index_from_basic_block(successor); + let index = index_from_basic_block(*successor); write!(f, "B{index}, ")?; } } if let Some(handler) = &basic_block.handler { - let index = index_from_basic_block(handler); + let index = index_from_basic_block(*handler); write!(f, " -- handler B{index}")?; } @@ -157,11 +156,11 @@ impl Debug for ControlFlowGraph { match terminator { Terminator::None => write!(f, "None")?, Terminator::JumpUnconditional { opcode, target } => { - let target = index_from_basic_block(target); + let target = index_from_basic_block(*target); write!(f, "{} B{target}", opcode.as_str())?; } Terminator::JumpConditional { opcode, no: _, yes } => { - let target = index_from_basic_block(yes); + let target = index_from_basic_block(*yes); write!(f, "{} B{target}", opcode.as_str())?; } Terminator::TemplateLookup { @@ -169,7 +168,7 @@ impl Debug for ControlFlowGraph { yes, site: _, } => { - let target = index_from_basic_block(yes); + let target = index_from_basic_block(*yes); write!(f, "TemplateLookup B{target}")?; } Terminator::Return => { @@ -252,13 +251,15 @@ impl ControlFlowGraph { /// TODO: doc #[must_use] - pub fn generate(bytecode: &[u8], handlers: &[Handler]) -> Self { + pub(crate) fn generate(bytecode: &[u8], handlers: &[Handler]) -> Self { let leaders = Self::leaders(bytecode, handlers); let block_count = leaders.len(); - let mut basic_blocks = IndexSet::with_capacity(block_count); + let mut basic_block_keys = Vec::with_capacity(block_count); + let mut basic_blocks = SlotMap::::with_capacity_and_key(block_count); for _ in 0..block_count { - basic_blocks.insert(RcBasicBlock::default()); + let key = basic_blocks.insert(BasicBlock::default()); + basic_block_keys.push(key); } let basic_block_from_bytecode_position = |address: u32| { @@ -267,7 +268,7 @@ impl ControlFlowGraph { .position(|x| *x == address) .expect("There should be a basic block"); - basic_blocks[index].clone() + basic_block_keys[index] }; let mut iter = InstructionIterator::new(bytecode); @@ -278,7 +279,7 @@ impl ControlFlowGraph { .skip(1) .map(|(i, leader)| (i - 1, leader)) { - let this = basic_blocks[i].clone(); + let key = basic_block_keys[i]; let handler = handlers .iter() @@ -287,7 +288,7 @@ impl ControlFlowGraph { if let Some(handler) = handler { let handler = basic_block_from_bytecode_position(handler.handler()); - this.borrow_mut().handler = Some(handler); + basic_blocks[key].handler = Some(handler); } let mut bytecode = Vec::new(); @@ -300,7 +301,7 @@ impl ControlFlowGraph { Instruction::Jump { address } | Instruction::Default { address } => { let target = basic_block_from_bytecode_position(address); - target.borrow_mut().predecessors.push(this.downgrade()); + basic_blocks[target].predecessors.push(key); terminator = Terminator::JumpUnconditional { opcode: instruction.opcode(), @@ -312,20 +313,20 @@ impl ControlFlowGraph { site, } => { let yes = basic_block_from_bytecode_position(address); - let no = basic_blocks[i + 1].clone(); + let no = basic_block_keys[i + 1]; - yes.borrow_mut().predecessors.push(this.downgrade()); - no.borrow_mut().predecessors.push(this.downgrade()); + basic_blocks[yes].predecessors.push(key); + basic_blocks[no].predecessors.push(key); terminator = Terminator::TemplateLookup { no, yes, site }; } instruction => { if let Some(address) = is_jump_kind_instruction(&instruction) { let yes = basic_block_from_bytecode_position(address); - let no = basic_blocks[i + 1].clone(); + let no = basic_block_keys[i + 1]; - yes.borrow_mut().predecessors.push(this.downgrade()); - no.borrow_mut().predecessors.push(this.downgrade()); + basic_blocks[yes].predecessors.push(key); + basic_blocks[no].predecessors.push(key); terminator = Terminator::JumpConditional { opcode: instruction.opcode(), @@ -343,36 +344,28 @@ impl ControlFlowGraph { } } - let mut basic_block = this.borrow_mut(); + let basic_block = &mut basic_blocks[key]; basic_block.instructions = bytecode; basic_block.terminator = terminator; } Self { - basic_block_start: basic_blocks[0].clone(), + basic_block_start: basic_block_keys[0], basic_blocks, } } /// Remove [`BasicBlock`]. - pub fn remove(&mut self, basic_block: &RcBasicBlock) { - self.basic_blocks.shift_remove(basic_block); - } - - /// Get [`BasicBlock`] index. - #[must_use] - pub fn get_index(&self, basic_block: &RcBasicBlock) -> usize { - self.basic_blocks - .get_index_of(basic_block) - .expect("there should be a BasicBlock in CFG") + pub(crate) fn remove(&mut self, basic_block: BasicBlockKey) { + self.basic_blocks.remove(basic_block); } /// Finalize bytecode. #[must_use] pub fn finalize(self) -> Vec { - let index_from_basic_block = |bb: &RcBasicBlock| { - for (i, basic_block) in self.basic_blocks.iter().enumerate() { - if Rc::ptr_eq(basic_block, bb) { + let index_from_basic_block = |bb: BasicBlockKey| { + for (i, key) in self.basic_blocks.keys().enumerate() { + if key == bb { return i; } } @@ -384,8 +377,8 @@ impl ControlFlowGraph { let mut labels = Vec::new(); let mut blocks = Vec::with_capacity(self.basic_blocks.len()); - for basic_block in &self.basic_blocks { - let basic_block = basic_block.borrow(); + for key in self.basic_blocks.keys() { + let basic_block = &self.basic_blocks[key]; blocks.push(results.len() as u32); @@ -400,7 +393,7 @@ impl ControlFlowGraph { let start = results.len(); results.extend_from_slice(&[0, 0, 0, 0]); - let target = index_from_basic_block(target); + let target = index_from_basic_block(*target); labels.push((start as u32, target)); } Terminator::JumpConditional { opcode, no: _, yes } => { @@ -408,7 +401,7 @@ impl ControlFlowGraph { let start = results.len(); results.extend_from_slice(&[0, 0, 0, 0]); - let target = index_from_basic_block(yes); + let target = index_from_basic_block(*yes); labels.push((start as u32, target)); } Terminator::TemplateLookup { yes, site, .. } => { @@ -417,7 +410,7 @@ impl ControlFlowGraph { results.extend_from_slice(&[0, 0, 0, 0]); results.extend_from_slice(&site.to_ne_bytes()); - let target = index_from_basic_block(yes); + let target = index_from_basic_block(*yes); labels.push((start as u32, target)); } Terminator::Return { .. } => { @@ -440,15 +433,6 @@ impl ControlFlowGraph { } } -impl Drop for ControlFlowGraph { - fn drop(&mut self) { - // NOTE: Untie BasicBlock nodes, so they can be deallocated. - for basic_block in &self.basic_blocks { - *basic_block.borrow_mut() = BasicBlock::default(); - } - } -} - /// Simplifies the [`ControlFlowGraph`]. /// /// # Operations @@ -460,30 +444,32 @@ pub struct GraphSimplification; impl GraphSimplification { /// TODO: doc - pub fn perform(graph: &mut ControlFlowGraph) -> bool { - let mut changed = false; - for basic_block_ptr in &graph.basic_blocks { - { - let mut basic_block = basic_block_ptr.borrow_mut(); - - #[allow(clippy::single_match)] - match basic_block.terminator.clone() { - Terminator::JumpConditional { no, yes, .. } => { - if no == yes { - basic_block.insert_last(Instruction::Pop); - basic_block.terminator = Terminator::JumpUnconditional { - opcode: Opcode::Jump, - target: yes, - }; - - changed |= true; - } - } - _ => {} - } - } - } - changed + pub fn perform(_graph: &mut ControlFlowGraph) -> bool { + // let mut changed = false; + + // for key in graph.basic_blocks.keys() { + // { + // let mut basic_block = basic_block_ptr.borrow_mut(); + + // #[allow(clippy::single_match)] + // match basic_block.terminator.clone() { + // Terminator::JumpConditional { no, yes, .. } => { + // if no == yes { + // basic_block.insert_last(Instruction::Pop); + // basic_block.terminator = Terminator::JumpUnconditional { + // opcode: Opcode::Jump, + // target: yes, + // }; + + // changed |= true; + // } + // } + // _ => {} + // } + // } + // } + // changed + false } } @@ -493,54 +479,55 @@ pub struct GraphEliminateUnreachableBasicBlocks; impl GraphEliminateUnreachableBasicBlocks { /// TODO: doc - pub fn perform(graph: &mut ControlFlowGraph) -> bool { - let mut changed = false; - - let mut stack = vec![graph.basic_block_start.clone()]; - while let Some(basic_block_ptr) = stack.pop() { - let mut basic_block = basic_block_ptr.borrow_mut(); - if basic_block.reachable() { - break; - } - basic_block.flags |= BasicBlockFlags::REACHABLE; - basic_block.next(&mut stack); - - // println!("{:p} -- {}", basic_block_ptr.as_ptr(), basic_block.reachable()); - } - - assert!( - graph.basic_block_start.borrow().reachable(), - "start basic block node should always be reachable" - ); - - let mut delete_list = Vec::new(); - for (i, basic_block) in graph.basic_blocks.iter().enumerate().rev() { - if !basic_block.borrow().reachable() { - delete_list.push(i); - } - } - - // println!("{delete_list:?}"); - - for i in delete_list { - let basic_block = graph - .basic_blocks - .shift_remove_index(i) - .expect("there should be a BasicBlock in CFG"); - let mut basic_block = basic_block.borrow_mut(); - - assert!( - !basic_block.reachable(), - "reachable basic blocks should not be eliminated" - ); - - basic_block.predecessors.clear(); - basic_block.terminator = Terminator::None; - - changed |= true; - } - - changed + pub fn perform(_graph: &mut ControlFlowGraph) -> bool { + // let mut changed = false; + + // let mut stack = vec![graph.basic_block_start.clone()]; + // while let Some(basic_block_ptr) = stack.pop() { + // let mut basic_block = basic_block_ptr.borrow_mut(); + // if basic_block.reachable() { + // break; + // } + // basic_block.flags |= BasicBlockFlags::REACHABLE; + // basic_block.next(&mut stack); + + // // println!("{:p} -- {}", basic_block_ptr.as_ptr(), basic_block.reachable()); + // } + + // assert!( + // graph.basic_block_start.borrow().reachable(), + // "start basic block node should always be reachable" + // ); + + // let mut delete_list = Vec::new(); + // for (i, basic_block) in graph.basic_blocks.iter().enumerate().rev() { + // if !basic_block.borrow().reachable() { + // delete_list.push(i); + // } + // } + + // // println!("{delete_list:?}"); + + // for i in delete_list { + // let basic_block = graph + // .basic_blocks + // .shift_remove_index(i) + // .expect("there should be a BasicBlock in CFG"); + // let mut basic_block = basic_block.borrow_mut(); + + // assert!( + // !basic_block.reachable(), + // "reachable basic blocks should not be eliminated" + // ); + + // basic_block.predecessors.clear(); + // basic_block.terminator = Terminator::None; + + // changed |= true; + // } + + // changed + false } } From 106accda9dd462271cbaef8064c3bdbcdc66dd75 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sat, 9 Dec 2023 16:40:59 +0100 Subject: [PATCH 4/5] Simplify naming and add end block --- .../control_flow_graph/basic_block.rs | 23 +++------ .../src/optimizer/control_flow_graph/mod.rs | 50 ++++++++++++------- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/core/engine/src/optimizer/control_flow_graph/basic_block.rs b/core/engine/src/optimizer/control_flow_graph/basic_block.rs index f66fb33dc39..4402b8a293e 100644 --- a/core/engine/src/optimizer/control_flow_graph/basic_block.rs +++ b/core/engine/src/optimizer/control_flow_graph/basic_block.rs @@ -16,7 +16,7 @@ bitflags! { /// TODO: doc #[derive(Default, Clone)] pub struct BasicBlock { - pub(crate) predecessors: Vec, + pub(crate) previous: Vec, pub(crate) instructions: Vec, pub(crate) terminator: Terminator, pub(crate) handler: Option, @@ -54,23 +54,15 @@ impl BasicBlock { self.flags.contains(BasicBlockFlags::REACHABLE) } - pub(crate) fn successors(&self) -> Vec { - match self.terminator { - Terminator::None => vec![], - Terminator::JumpUnconditional { target, .. } => { - vec![target] - } - Terminator::JumpConditional { no, yes, .. } - | Terminator::TemplateLookup { no, yes, .. } => { - vec![no, yes] - } - Terminator::Return => Vec::new(), - } + pub(crate) fn next(&self) -> Vec { + let mut result = Vec::new(); + self.next_into(&mut result); + result } - pub(crate) fn next(&self, nexts: &mut Vec) { + pub(crate) fn next_into(&self, nexts: &mut Vec) { match self.terminator { - Terminator::None | Terminator::Return => {} + Terminator::None => {} Terminator::JumpUnconditional { target, .. } => { nexts.push(target); } @@ -79,6 +71,7 @@ impl BasicBlock { nexts.push(no); nexts.push(yes); } + Terminator::Return { end } => nexts.push(end), } } } diff --git a/core/engine/src/optimizer/control_flow_graph/mod.rs b/core/engine/src/optimizer/control_flow_graph/mod.rs index 2162dff3bdd..352e075d06c 100644 --- a/core/engine/src/optimizer/control_flow_graph/mod.rs +++ b/core/engine/src/optimizer/control_flow_graph/mod.rs @@ -54,7 +54,7 @@ pub(crate) enum Terminator { }, /// TODO: doc - Return, + Return { end: BasicBlockKey }, } impl Terminator { @@ -89,6 +89,7 @@ impl Terminator { /// TODO: doc pub struct ControlFlowGraph { basic_block_start: BasicBlockKey, + basic_block_end: BasicBlockKey, basic_blocks: SlotMap, } @@ -122,19 +123,26 @@ impl Debug for ControlFlowGraph { if basic_block.reachable() { "" } else { "not " } )?; - if !basic_block.predecessors.is_empty() { - write!(f, " -- predecessors ")?; - for predecessor in &basic_block.predecessors { + if key == self.basic_block_start { + write!(f, " -- start")?; + } + if key == self.basic_block_end { + write!(f, " -- end")?; + } + + if !basic_block.previous.is_empty() { + write!(f, " -- previous ")?; + for predecessor in &basic_block.previous { let index = index_from_basic_block(*predecessor); write!(f, "B{index}, ")?; } } - let successors = basic_block.successors(); - if !successors.is_empty() { - write!(f, " -- successors ")?; - for successor in &successors { - let index = index_from_basic_block(*successor); + let next = basic_block.next(); + if !next.is_empty() { + write!(f, " -- next ")?; + for bb in &next { + let index = index_from_basic_block(*bb); write!(f, "B{index}, ")?; } } @@ -171,8 +179,9 @@ impl Debug for ControlFlowGraph { let target = index_from_basic_block(*yes); write!(f, "TemplateLookup B{target}")?; } - Terminator::Return => { - write!(f, "Return")?; + Terminator::Return { end } => { + let target = index_from_basic_block(*end); + write!(f, "Return B{target}")?; } } writeln!(f)?; @@ -271,6 +280,8 @@ impl ControlFlowGraph { basic_block_keys[index] }; + let basic_block_end = basic_block_keys[leaders.len() - 1]; + let mut iter = InstructionIterator::new(bytecode); for (i, leader) in leaders .iter() @@ -296,12 +307,14 @@ impl ControlFlowGraph { while let Some((_, _, instruction)) = iter.next() { match instruction { Instruction::Return => { - terminator = Terminator::Return; + terminator = Terminator::Return { + end: basic_block_end, + }; } Instruction::Jump { address } | Instruction::Default { address } => { let target = basic_block_from_bytecode_position(address); - basic_blocks[target].predecessors.push(key); + basic_blocks[target].previous.push(key); terminator = Terminator::JumpUnconditional { opcode: instruction.opcode(), @@ -315,8 +328,8 @@ impl ControlFlowGraph { let yes = basic_block_from_bytecode_position(address); let no = basic_block_keys[i + 1]; - basic_blocks[yes].predecessors.push(key); - basic_blocks[no].predecessors.push(key); + basic_blocks[yes].previous.push(key); + basic_blocks[no].previous.push(key); terminator = Terminator::TemplateLookup { no, yes, site }; } @@ -325,8 +338,8 @@ impl ControlFlowGraph { let yes = basic_block_from_bytecode_position(address); let no = basic_block_keys[i + 1]; - basic_blocks[yes].predecessors.push(key); - basic_blocks[no].predecessors.push(key); + basic_blocks[yes].previous.push(key); + basic_blocks[no].previous.push(key); terminator = Terminator::JumpConditional { opcode: instruction.opcode(), @@ -351,6 +364,7 @@ impl ControlFlowGraph { Self { basic_block_start: basic_block_keys[0], + basic_block_end, basic_blocks, } } @@ -520,7 +534,7 @@ impl GraphEliminateUnreachableBasicBlocks { // "reachable basic blocks should not be eliminated" // ); - // basic_block.predecessors.clear(); + // basic_block.previous.clear(); // basic_block.terminator = Terminator::None; // changed |= true; From 126367fda60cbc90568573ac976d2d6b9f492aae Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sat, 9 Dec 2023 19:45:09 +0100 Subject: [PATCH 5/5] Add JumpTable as Terminator --- .../control_flow_graph/basic_block.rs | 23 +++-- .../src/optimizer/control_flow_graph/mod.rs | 88 ++++++++++++++++++- 2 files changed, 100 insertions(+), 11 deletions(-) diff --git a/core/engine/src/optimizer/control_flow_graph/basic_block.rs b/core/engine/src/optimizer/control_flow_graph/basic_block.rs index 4402b8a293e..a6bcd801817 100644 --- a/core/engine/src/optimizer/control_flow_graph/basic_block.rs +++ b/core/engine/src/optimizer/control_flow_graph/basic_block.rs @@ -2,7 +2,7 @@ use std::hash::Hash; use bitflags::bitflags; -use crate::vm::Instruction; +use crate::vm::{Handler, Instruction}; use super::{BasicBlockKey, Terminator}; @@ -13,13 +13,18 @@ bitflags! { } } +pub(crate) struct BasicBlockHandler { + basic_block: BasicBlockKey, + handler: Handler, +} + /// TODO: doc #[derive(Default, Clone)] pub struct BasicBlock { pub(crate) previous: Vec, pub(crate) instructions: Vec, pub(crate) terminator: Terminator, - pub(crate) handler: Option, + pub(crate) handler: Option<(BasicBlockKey, Handler)>, pub(crate) flags: BasicBlockFlags, } @@ -61,17 +66,21 @@ impl BasicBlock { } pub(crate) fn next_into(&self, nexts: &mut Vec) { - match self.terminator { + match &self.terminator { Terminator::None => {} Terminator::JumpUnconditional { target, .. } => { - nexts.push(target); + nexts.push(*target); } Terminator::JumpConditional { no, yes, .. } | Terminator::TemplateLookup { no, yes, .. } => { - nexts.push(no); - nexts.push(yes); + nexts.push(*no); + nexts.push(*yes); + } + Terminator::JumpTable { default, addresses } => { + nexts.push(*default); + nexts.extend_from_slice(addresses); } - Terminator::Return { end } => nexts.push(end), + Terminator::Return { end } => nexts.push(*end), } } } diff --git a/core/engine/src/optimizer/control_flow_graph/mod.rs b/core/engine/src/optimizer/control_flow_graph/mod.rs index 352e075d06c..0c322cf8fbf 100644 --- a/core/engine/src/optimizer/control_flow_graph/mod.rs +++ b/core/engine/src/optimizer/control_flow_graph/mod.rs @@ -9,15 +9,17 @@ use std::fmt::Debug; use rustc_hash::FxHashMap; use slotmap::{new_key_type, SlotMap}; +use thin_vec::ThinVec; use crate::vm::{CodeBlock, Handler, Instruction, InstructionIterator, Opcode}; pub use self::basic_block::BasicBlock; new_key_type! { pub(crate) struct BasicBlockKey; } +new_key_type! { pub(crate) struct HandlerKey; } /// TODO: doc -#[derive(Default, Clone, Copy)] +#[derive(Default, Clone)] pub(crate) enum Terminator { /// TODO: doc #[default] @@ -53,6 +55,11 @@ pub(crate) enum Terminator { site: u64, }, + JumpTable { + default: BasicBlockKey, + addresses: ThinVec, + }, + /// TODO: doc Return { end: BasicBlockKey }, } @@ -91,6 +98,7 @@ pub struct ControlFlowGraph { basic_block_start: BasicBlockKey, basic_block_end: BasicBlockKey, basic_blocks: SlotMap, + // handlers: SlotMap, } impl Debug for ControlFlowGraph { @@ -148,7 +156,7 @@ impl Debug for ControlFlowGraph { } if let Some(handler) = &basic_block.handler { - let index = index_from_basic_block(*handler); + let index = index_from_basic_block(handler.0); write!(f, " -- handler B{index}")?; } @@ -179,6 +187,21 @@ impl Debug for ControlFlowGraph { let target = index_from_basic_block(*yes); write!(f, "TemplateLookup B{target}")?; } + Terminator::JumpTable { default, addresses } => { + let default = index_from_basic_block(*default); + write!(f, "JumpTable default: B{default}")?; + + if !addresses.is_empty() { + write!(f, " ")?; + for (i, address) in addresses.iter().enumerate() { + let address = index_from_basic_block(*address); + write!(f, "{}: B{address}", i + 1)?; + if i + 1 != addresses.len() { + write!(f, ", ")?; + } + } + } + } Terminator::Return { end } => { let target = index_from_basic_block(*end); write!(f, "Return B{target}")?; @@ -234,6 +257,10 @@ impl ControlFlowGraph { leaders.push(iter.pc() as u32); leaders.push(exit); } + Instruction::JumpTable { default, addresses } => { + leaders.push(default); + leaders.extend_from_slice(&addresses); + } instruction => { if let Some(target) = is_jump_kind_instruction(&instruction) { leaders.push(iter.pc() as u32); @@ -297,9 +324,9 @@ impl ControlFlowGraph { .rev() .find(|handler| handler.contains(iter.pc() as u32)); if let Some(handler) = handler { - let handler = basic_block_from_bytecode_position(handler.handler()); + let basic_block_handler = basic_block_from_bytecode_position(handler.handler()); - basic_blocks[key].handler = Some(handler); + basic_blocks[key].handler = Some((basic_block_handler, *handler)); } let mut bytecode = Vec::new(); @@ -333,6 +360,23 @@ impl ControlFlowGraph { terminator = Terminator::TemplateLookup { no, yes, site }; } + Instruction::JumpTable { default, addresses } => { + let default = basic_block_from_bytecode_position(default); + + basic_blocks[default].previous.push(key); + + let mut basic_block_addresses = ThinVec::with_capacity(addresses.len()); + for address in addresses { + let address = basic_block_from_bytecode_position(address); + basic_blocks[address].previous.push(key); + basic_block_addresses.push(address); + } + + terminator = Terminator::JumpTable { + default, + addresses: basic_block_addresses, + } + } instruction => { if let Some(address) = is_jump_kind_instruction(&instruction) { let yes = basic_block_from_bytecode_position(address); @@ -427,6 +471,24 @@ impl ControlFlowGraph { let target = index_from_basic_block(*yes); labels.push((start as u32, target)); } + Terminator::JumpTable { default, addresses } => { + results.extend_from_slice(&[Opcode::JumpTable as u8]); + let start = results.len(); + results.extend_from_slice(&[0, 0, 0, 0]); + + let default = index_from_basic_block(*default); + labels.push((start as u32, default)); + + results.extend_from_slice(&(addresses.len() as u32).to_ne_bytes()); + + for address in addresses { + let start = results.len(); + results.extend_from_slice(&[0, 0, 0, 0]); + + let target = index_from_basic_block(*address); + labels.push((start as u32, target)); + } + } Terminator::Return { .. } => { results.push(Opcode::Return as u8); } @@ -561,4 +623,22 @@ mod test { assert_eq!(bytecode, actual.as_slice()); } + + #[test] + fn preserve_jump_table() { + let bytecode = &[ + 193, 68, 0, 18, 67, 1, 72, 1, 140, 1, 76, 153, 2, 18, 155, 6, 17, 118, 38, 0, 0, 0, + 155, 5, 17, 118, 38, 0, 0, 0, 126, 16, 118, 38, 0, 0, 0, 16, 151, 153, 3, 18, 71, 1, + 143, 0, 152, 155, 152, 120, 55, 0, 0, 0, 124, 123, 71, 0, 0, 0, 1, 0, 0, 0, 68, 0, 0, + 0, 152, 147, 148, 18, 152, 147, 148, + ]; + + let graph = ControlFlowGraph::generate(bytecode, &[]); + + println!("{graph:?}"); + + let actual = graph.finalize(); + + assert_eq!(bytecode, actual.as_slice()); + } }