feat: Add dead code elimination and improve optimization
Integrates dead code elimination (DCE) into the optimizer. This phase removes expressions that have no side effects and are not the result of the block. Also includes several improvements to the existing optimization passes: - DCE now correctly handles assignments to variables that are never used or captured. - Explicitly tracks captured slots in the `SubstitutionMap` to prevent premature inlining. - Introduces a `is_side_effect_free` helper function for more robust purity checks. - Updates dependencies to include `insta` for snapshot testing.
This commit is contained in:
Generated
+38
@@ -749,6 +749,18 @@ dependencies = [
|
||||
"crossbeam-utils",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "console"
|
||||
version = "0.15.11"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "054ccb5b10f9f2cbf51eb355ca1d05c2d279ce1804688d0db74b4733a5aeafd8"
|
||||
dependencies = [
|
||||
"encode_unicode",
|
||||
"libc",
|
||||
"once_cell",
|
||||
"windows-sys 0.59.0",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "core-foundation"
|
||||
version = "0.9.4"
|
||||
@@ -1025,6 +1037,12 @@ dependencies = [
|
||||
"bytemuck",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "encode_unicode"
|
||||
version = "1.0.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0"
|
||||
|
||||
[[package]]
|
||||
name = "endi"
|
||||
version = "1.1.1"
|
||||
@@ -1660,6 +1678,19 @@ dependencies = [
|
||||
"serde_core",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "insta"
|
||||
version = "1.46.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e82db8c87c7f1ccecb34ce0c24399b8a73081427f3c7c50a5d597925356115e4"
|
||||
dependencies = [
|
||||
"console",
|
||||
"once_cell",
|
||||
"serde",
|
||||
"similar",
|
||||
"tempfile",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "is_terminal_polyfill"
|
||||
version = "1.70.2"
|
||||
@@ -1884,6 +1915,7 @@ dependencies = [
|
||||
"chrono",
|
||||
"clap",
|
||||
"eframe",
|
||||
"insta",
|
||||
"regex",
|
||||
]
|
||||
|
||||
@@ -2760,6 +2792,12 @@ version = "0.3.8"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e320a6c5ad31d271ad523dcf3ad13e2767ad8b1cb8f047f75a8aeaf8da139da2"
|
||||
|
||||
[[package]]
|
||||
name = "similar"
|
||||
version = "2.7.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa"
|
||||
|
||||
[[package]]
|
||||
name = "slab"
|
||||
version = "0.4.12"
|
||||
|
||||
@@ -9,3 +9,6 @@ eframe = "0.33.3"
|
||||
clap = { version = "4.5", features = ["derive"] }
|
||||
chrono = "0.4"
|
||||
regex = "1.10"
|
||||
|
||||
[dev-dependencies]
|
||||
insta = { version = "1.39", features = ["yaml"] }
|
||||
|
||||
+109
-11
@@ -3,9 +3,9 @@ use crate::ast::compiler::bound_nodes::{BoundKind, TypedNode, Address};
|
||||
use crate::ast::types::{Value, StaticType};
|
||||
use crate::ast::vm::Closure;
|
||||
use crate::ast::nodes::Node;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
|
||||
/// The Optimizer performs Phase 2 (Cracking) and Phase 2.5 (Aggressive Collapsing).
|
||||
/// The Optimizer performs Phase 2 (Cracking) and Phase 2.5/2.6 (Aggressive Collapsing & DCE).
|
||||
pub struct Optimizer {
|
||||
pub level: u32,
|
||||
max_passes: usize,
|
||||
@@ -45,6 +45,7 @@ impl Optimizer {
|
||||
kind: BoundKind::Constant(val.clone()),
|
||||
};
|
||||
}
|
||||
sub.used_slots.insert(slot);
|
||||
}
|
||||
Address::Upvalue(idx) => {
|
||||
if let Some(val) = sub.upvalues.get(&idx) {
|
||||
@@ -148,6 +149,33 @@ impl Optimizer {
|
||||
}
|
||||
|
||||
if self.level >= 2 {
|
||||
// Phase 2.6: Dead Code Elimination (DCE)
|
||||
// Remove side-effect-free expressions from the block that are not the result.
|
||||
if !new_exprs.is_empty() {
|
||||
let last_idx = new_exprs.len() - 1;
|
||||
let mut filtered = Vec::with_capacity(new_exprs.len());
|
||||
for (i, e) in new_exprs.into_iter().enumerate() {
|
||||
if i == last_idx {
|
||||
filtered.push(e);
|
||||
continue;
|
||||
}
|
||||
|
||||
// A statement is removable if it's pure (no side effects)
|
||||
// or if it's a local assignment to a variable that is never used or captured.
|
||||
let removable = match &e.kind {
|
||||
BoundKind::DefLocal { slot, .. } | BoundKind::Set { addr: Address::Local(slot), .. } => {
|
||||
!sub.used_slots.contains(slot) && !sub.captured_slots.contains(slot) && self.is_side_effect_free(&e)
|
||||
}
|
||||
_ => self.is_side_effect_free(&e),
|
||||
};
|
||||
|
||||
if !removable {
|
||||
filtered.push(e);
|
||||
}
|
||||
}
|
||||
new_exprs = filtered;
|
||||
}
|
||||
|
||||
if new_exprs.is_empty() {
|
||||
return Node { identity: node.identity, kind: BoundKind::Nop, ty: StaticType::Void };
|
||||
} else if new_exprs.len() == 1 {
|
||||
@@ -169,6 +197,8 @@ impl Optimizer {
|
||||
match capture_addr {
|
||||
Address::Local(slot) => {
|
||||
if let Some(val) = sub.locals.get(slot) { inlined_val = Some(val.clone()); }
|
||||
// Mark slot as captured in parent scope
|
||||
sub.captured_slots.insert(*slot);
|
||||
}
|
||||
Address::Upvalue(idx) => {
|
||||
if let Some(val) = sub.upvalues.get(idx) { inlined_val = Some(val.clone()); }
|
||||
@@ -203,6 +233,9 @@ impl Optimizer {
|
||||
},
|
||||
|
||||
BoundKind::DefLocal { name, slot, value, captured_by } => {
|
||||
if !captured_by.is_empty() {
|
||||
sub.captured_slots.insert(slot);
|
||||
}
|
||||
let value = Box::new(self.visit_node(*value, sub));
|
||||
if let BoundKind::Constant(val) = &value.kind {
|
||||
sub.add_local(slot, val.clone());
|
||||
@@ -244,6 +277,21 @@ impl Optimizer {
|
||||
Node { identity: node.identity, kind: new_kind, ty: new_ty }
|
||||
}
|
||||
|
||||
fn is_side_effect_free(&self, node: &TypedNode) -> bool {
|
||||
match &node.kind {
|
||||
BoundKind::Constant(_) | BoundKind::Get { .. } | BoundKind::Parameter { .. } | BoundKind::Nop | BoundKind::Lambda { .. } => true,
|
||||
BoundKind::Tuple { elements } => elements.iter().all(|e| self.is_side_effect_free(e)),
|
||||
BoundKind::Record { fields } => fields.iter().all(|(k, v)| self.is_side_effect_free(k) && self.is_side_effect_free(v)),
|
||||
BoundKind::If { cond, then_br, else_br } => {
|
||||
self.is_side_effect_free(cond) && self.is_side_effect_free(then_br) && else_br.as_ref().is_none_or(|e| self.is_side_effect_free(e))
|
||||
}
|
||||
BoundKind::Block { exprs } => exprs.iter().all(|e| self.is_side_effect_free(e)),
|
||||
BoundKind::Expansion { bound_expanded, .. } => self.is_side_effect_free(bound_expanded),
|
||||
BoundKind::DefLocal { value, .. } | BoundKind::Set { value, .. } => self.is_side_effect_free(value),
|
||||
_ => false, // Call and DefGlobal are considered impure
|
||||
}
|
||||
}
|
||||
|
||||
fn try_beta_reduce(&self, params: &TypedNode, args: &TypedNode, body: TypedNode) -> Option<TypedNode> {
|
||||
if self.contains_def_local(&body) {
|
||||
return None;
|
||||
@@ -354,11 +402,20 @@ struct SubstitutionMap {
|
||||
locals: HashMap<u32, Value>,
|
||||
/// Mapping Address::Upvalue(idx) -> Value
|
||||
upvalues: HashMap<u32, Value>,
|
||||
/// Tracking which local slots are actually used (not inlined)
|
||||
used_slots: HashSet<u32>,
|
||||
/// Tracking which local slots are captured by lambdas
|
||||
captured_slots: HashSet<u32>,
|
||||
}
|
||||
|
||||
impl SubstitutionMap {
|
||||
fn new() -> Self {
|
||||
Self { locals: HashMap::new(), upvalues: HashMap::new() }
|
||||
Self {
|
||||
locals: HashMap::new(),
|
||||
upvalues: HashMap::new(),
|
||||
used_slots: HashSet::new(),
|
||||
captured_slots: HashSet::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn add_local(&mut self, slot: u32, val: Value) {
|
||||
@@ -376,7 +433,7 @@ impl SubstitutionMap {
|
||||
if let Some(res) = mapping.get(idx as usize) {
|
||||
match res {
|
||||
Some(new_idx) => (BoundKind::Get { addr: Address::Upvalue(*new_idx), name }, node.ty),
|
||||
None => (BoundKind::Get { addr: Address::Upvalue(idx), name }, node.ty), // Should have been inlined
|
||||
None => (BoundKind::Get { addr: Address::Upvalue(idx), name }, node.ty),
|
||||
}
|
||||
} else {
|
||||
(BoundKind::Get { addr: Address::Upvalue(idx), name }, node.ty)
|
||||
@@ -384,8 +441,6 @@ impl SubstitutionMap {
|
||||
},
|
||||
|
||||
BoundKind::Lambda { params, upvalues, body, positional_count } => {
|
||||
// IMPORTANT: If this nested lambda captures an upvalue from our current scope,
|
||||
// we MUST re-index it in its own capture list!
|
||||
let mut next_upvalues = Vec::new();
|
||||
for addr in upvalues {
|
||||
if let Address::Upvalue(idx) = addr
|
||||
@@ -393,18 +448,14 @@ impl SubstitutionMap {
|
||||
if let Some(new_idx) = res {
|
||||
next_upvalues.push(Address::Upvalue(*new_idx));
|
||||
}
|
||||
continue; // Inlined or re-indexed
|
||||
continue;
|
||||
}
|
||||
next_upvalues.push(addr);
|
||||
}
|
||||
|
||||
// Note: We don't recurse into the body with the SAME mapping,
|
||||
// because nested Get(Upvalue) nodes refer to THIS lambda's capture list.
|
||||
(BoundKind::Lambda { params, upvalues: next_upvalues, body, positional_count }, node.ty)
|
||||
},
|
||||
|
||||
BoundKind::If { cond, then_br, else_br } => {
|
||||
|
||||
let cond = Box::new(self.reindex_upvalues(*cond, mapping));
|
||||
let then_br = Box::new(self.reindex_upvalues(*then_br, mapping));
|
||||
let else_br = else_br.map(|e| Box::new(self.reindex_upvalues(*e, mapping)));
|
||||
@@ -441,3 +492,50 @@ impl SubstitutionMap {
|
||||
Node { identity: node.identity, kind: new_kind, ty: new_ty }
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::ast::environment::Environment;
|
||||
|
||||
fn get_optimized_dump(source: &str, level: u32) -> String {
|
||||
let mut env = Environment::new();
|
||||
env.optimization_level = level;
|
||||
env.dump_ast(source).expect("Compilation failed during test")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opt_folding() {
|
||||
let dump = get_optimized_dump("(+ 10 20)", 2);
|
||||
insta::assert_snapshot!(dump);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opt_local_inlining() {
|
||||
// Wrap in a do-block within a lambda to force DefLocal
|
||||
let source = "(fn [] (do (def x 10) (+ x 5)))";
|
||||
let dump = get_optimized_dump(source, 2);
|
||||
insta::assert_snapshot!(dump);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opt_dce_unused() {
|
||||
let source = "(fn [] (do (def x 10) 42))";
|
||||
let dump = get_optimized_dump(source, 2);
|
||||
insta::assert_snapshot!(dump);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opt_assignment_safety() {
|
||||
let source = "(fn [] (do (def x 10) (assign x 20) x))";
|
||||
let dump = get_optimized_dump(source, 2);
|
||||
insta::assert_snapshot!(dump);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_opt_lambda_cracking() {
|
||||
let source = "(fn [] (do (def x 10) (fn [] x)))";
|
||||
let dump = get_optimized_dump(source, 2);
|
||||
insta::assert_snapshot!(dump);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user