From f15ff43df8adf45f6cdbf91b65095656986507a8 Mon Sep 17 00:00:00 2001 From: Jesse Brault Date: Fri, 3 Jul 2026 15:29:07 -0500 Subject: [PATCH] Fix test for overlapping register assignments when k=2. --- dmc-lib/src/ir/ir_block.rs | 45 ------------- dmc-lib/src/ir/ir_function.rs | 6 +- dmc-lib/src/ir/mod.rs | 19 +----- dmc-lib/src/ir/register_allocation.rs | 65 +++++++++++++++++-- dmc-lib/src/ir/variable_locations.rs | 2 +- .../src/semantic_analysis/collect_scopes.rs | 3 + .../src/semantic_analysis/resolve_names.rs | 7 +- .../semantic_analysis/resolve_types/mod.rs | 3 + 8 files changed, 77 insertions(+), 73 deletions(-) diff --git a/dmc-lib/src/ir/ir_block.rs b/dmc-lib/src/ir/ir_block.rs index 8d81539..911b0c8 100644 --- a/dmc-lib/src/ir/ir_block.rs +++ b/dmc-lib/src/ir/ir_block.rs @@ -74,48 +74,3 @@ impl VrUser for IrBlock { // Ok(()) // } // } - -#[cfg(test)] -mod tests { - use crate::diagnostic::Diagnostic; - use crate::offset_counter::OffsetCounter; - use crate::parser::get_compilation_unit; - use crate::symbol_table::SymbolTable; - use crate::types_table::TypesTable; - - #[test] - fn overlapping_assignments_bug_when_k_2() -> Result<(), Vec> { - let mut compilation_unit = get_compilation_unit( - " - fn main() - let a = 1 - let b = 2 - let c = 3 - let x = a + b + c - end - ", - None, - )?; - - let mut symbol_table = SymbolTable::new(); - let mut types_table = TypesTable::new(); - - compilation_unit.init_scopes(&mut symbol_table); - compilation_unit.gather_symbols_into(&mut symbol_table)?; - compilation_unit.check_names(&mut symbol_table)?; - compilation_unit.gather_types_into(&symbol_table, &mut types_table)?; - compilation_unit.type_check(&symbol_table, &mut types_table)?; - - let main = compilation_unit - .functions() - .iter() - .find(|f| f.declared_name() == "main") - .unwrap(); - - let main_ir = main.to_ir(&symbol_table, &types_table, None); - let variable_locations = main_ir.assign_registers(2); - assert_eq!(variable_locations.register_variables_count(), 4); - - Ok(()) - } -} diff --git a/dmc-lib/src/ir/ir_function.rs b/dmc-lib/src/ir/ir_function.rs index 3082e28..29e0f7b 100644 --- a/dmc-lib/src/ir/ir_function.rs +++ b/dmc-lib/src/ir/ir_function.rs @@ -4,7 +4,6 @@ use crate::ir::ir_block::IrBlock; use crate::ir::ir_parameter::IrParameter; use crate::ir::ir_type_info::{IrTypeInfo, IrTypeInfoId}; use crate::ir::ir_variable::{IrVariable, IrVariableId}; -use crate::ir::register_allocation::block_assign_registers; use crate::ir::variable_locations::VariableLocations; use dvm_lib::vm::function::Function; use std::collections::HashMap; @@ -64,7 +63,8 @@ impl IrFunction { unimplemented!("having more than one block in a function is not yet implemented.") } let block = &self.blocks[0]; - block_assign_registers(block, register_count) + //block_assign_registers(block, register_count) + todo!() } #[deprecated] @@ -79,7 +79,7 @@ impl IrFunction { Function::new( self.fqn.clone(), self.parameters.len(), - variable_locations.stack_size(), + variable_locations.stack_variables_count(), instructions, ) } diff --git a/dmc-lib/src/ir/mod.rs b/dmc-lib/src/ir/mod.rs index 6f8cf84..8e7254c 100644 --- a/dmc-lib/src/ir/mod.rs +++ b/dmc-lib/src/ir/mod.rs @@ -1,12 +1,8 @@ use crate::constants_table::ConstantsTable; use crate::ir::assemble::assemble_ir_function; use crate::ir::ir_function::IrFunction; -use crate::ir::ir_type_info::{IrTypeInfo, IrTypeInfoId}; -use crate::ir::ir_variable::IrVariableId; -use crate::ir::register_allocation::block_assign_registers; -use crate::ir::variable_locations::VariableLocations; +use crate::ir::register_allocation::assign_registers; use dvm_lib::vm::function::Function; -use std::collections::HashMap; mod assemble; mod debug_print; @@ -43,18 +39,7 @@ pub fn compile_dvm_function( Function::new( ir_function.fqn().into(), ir_function.parameters().len(), - variable_locations.stack_size(), + variable_locations.stack_variables_count(), instructions, ) } - -fn assign_registers(ir_function: &IrFunction, register_count: usize) -> VariableLocations { - if ir_function.blocks().is_empty() { - return VariableLocations::new(HashMap::new(), HashMap::new()); - } - if ir_function.blocks().len() > 1 { - unimplemented!("having more than one block in a function is not yet implemented.") - } - let block = &ir_function.blocks()[0]; - block_assign_registers(block, register_count) -} diff --git a/dmc-lib/src/ir/register_allocation.rs b/dmc-lib/src/ir/register_allocation.rs index a8fcf76..b9d97a6 100644 --- a/dmc-lib/src/ir/register_allocation.rs +++ b/dmc-lib/src/ir/register_allocation.rs @@ -1,6 +1,7 @@ /// Used this video as source for algorithms in this module: /// https://www.youtube.com/watch?v=eWp_-XCwN1A use crate::ir::ir_block::IrBlock; +use crate::ir::ir_function::IrFunction; use crate::ir::ir_variable::IrVariableId; use crate::ir::variable_locations::VariableLocations; use dvm_lib::instruction::{Register, StackFrameOffset}; @@ -10,7 +11,18 @@ pub type RegisterAssignment = Register; pub type InterferenceGraph = HashMap>; pub type LivenessSets = Vec>; -pub fn block_live_in_live_out(ir_block: &IrBlock) -> (LivenessSets, LivenessSets) { +pub fn assign_registers(ir_function: &IrFunction, register_count: usize) -> VariableLocations { + if ir_function.blocks().is_empty() { + return VariableLocations::new(HashMap::new(), HashMap::new()); + } + if ir_function.blocks().len() > 1 { + unimplemented!("having more than one block in a function is not yet implemented.") + } + let block = &ir_function.blocks()[0]; + block_assign_registers(block, register_count) +} + +fn block_live_in_live_out(ir_block: &IrBlock) -> (LivenessSets, LivenessSets) { // init let n_statements = ir_block.statements().len(); let mut live_in: LivenessSets = vec![HashSet::new(); n_statements]; @@ -56,7 +68,7 @@ pub fn block_live_in_live_out(ir_block: &IrBlock) -> (LivenessSets, LivenessSets (live_in, live_out) } -pub fn block_interference_graph( +fn block_interference_graph( ir_block: &IrBlock, spilled: &HashSet, ) -> InterferenceGraph { @@ -74,8 +86,8 @@ pub fn block_interference_graph( // create graph and init all variables' outgoing-edge sets let mut graph: InterferenceGraph = HashMap::new(); - for variable in all_vr_variables { - graph.insert(variable, HashSet::new()); + for variable in &all_vr_variables { + graph.insert(*variable, HashSet::new()); } let (_, live_out) = block_live_in_live_out(ir_block); @@ -89,6 +101,10 @@ pub fn block_interference_graph( } for live_out_variable in statement_live_out { + // check that we aren't working with an out variable we have already spilled. + if spilled.contains(live_out_variable) { + continue; + } // we do the following check to avoid adding an edge to itself if definition_vr_variable != *live_out_variable { // add edges to both sets (two-way) @@ -104,7 +120,7 @@ pub fn block_interference_graph( graph } -pub fn block_assign_registers(ir_block: &IrBlock, register_count: usize) -> VariableLocations { +fn block_assign_registers(ir_block: &IrBlock, register_count: usize) -> VariableLocations { let mut spilled: HashSet = HashSet::new(); loop { let mut interference_graph = block_interference_graph(ir_block, &spilled); @@ -303,6 +319,10 @@ fn can_optimistically_color( #[cfg(test)] mod tests { use super::*; + use crate::diagnostic::Diagnostics; + use crate::lowering::lower_to_ir; + use crate::parser::get_compilation_unit; + use crate::semantic_analysis::analyze_compilation_unit; fn line_graph() -> InterferenceGraph { let mut graph: InterferenceGraph = HashMap::new(); @@ -417,4 +437,39 @@ mod tests { assert_eq!(registers.len(), 2); assert_eq!(spills.len(), 1); } + + #[test] + fn overlapping_assignments_bug_when_k_2() -> Result<(), Diagnostics> { + let compilation_unit = get_compilation_unit( + " + fn main() + let a = 1 + let b = 2 + let c = 3 + let x = a + b + c + end + ", + None, + )?; + + let (analysis_result, diagnostics) = analyze_compilation_unit(&compilation_unit); + assert!(diagnostics.is_empty()); + + let lower_result = lower_to_ir( + &compilation_unit, + &analysis_result.symbols, + &analysis_result.nodes_to_symbols, + &analysis_result.type_infos, + &analysis_result.symbols_to_type_infos, + &analysis_result.nodes_to_type_infos, + ); // todo: make this API friendlier + + assert_eq!(lower_result.functions.len(), 1); + let main_fn = &lower_result.functions[0]; + + let variable_locations = assign_registers(main_fn, 2); + assert_eq!(variable_locations.register_variables_count(), 4); + + Ok(()) + } } diff --git a/dmc-lib/src/ir/variable_locations.rs b/dmc-lib/src/ir/variable_locations.rs index 56e107c..386fcd9 100644 --- a/dmc-lib/src/ir/variable_locations.rs +++ b/dmc-lib/src/ir/variable_locations.rs @@ -36,7 +36,7 @@ impl VariableLocations { self.register_variables.len() } - pub fn stack_size(&self) -> usize { + pub fn stack_variables_count(&self) -> usize { self.stack_variables.len() } } diff --git a/dmc-lib/src/semantic_analysis/collect_scopes.rs b/dmc-lib/src/semantic_analysis/collect_scopes.rs index 722068e..0a55587 100644 --- a/dmc-lib/src/semantic_analysis/collect_scopes.rs +++ b/dmc-lib/src/semantic_analysis/collect_scopes.rs @@ -144,6 +144,9 @@ fn collect_scopes_statement(statement: &Statement, ctx: &mut ScopeCollectionCont fn collect_scopes_let_statement(let_statement: &LetStatement, ctx: &mut ScopeCollectionContext) { collect_scopes_expression(let_statement.initializer(), ctx); + let current_scope_id = ctx.current_scope_id().unwrap(); + ctx.nodes_to_scopes_mut() + .insert(let_statement.node_id(), current_scope_id); } fn collect_scopes_expression_statement( diff --git a/dmc-lib/src/semantic_analysis/resolve_names.rs b/dmc-lib/src/semantic_analysis/resolve_names.rs index cf04f6c..a299c06 100644 --- a/dmc-lib/src/semantic_analysis/resolve_names.rs +++ b/dmc-lib/src/semantic_analysis/resolve_names.rs @@ -1,4 +1,3 @@ -use crate::ast::NodeId; use crate::ast::assign_statement::AssignStatement; use crate::ast::binary_expression::BinaryExpression; use crate::ast::call::Call; @@ -11,10 +10,11 @@ use crate::ast::let_statement::LetStatement; use crate::ast::negative_expression::NegativeExpression; use crate::ast::parameter::Parameter; use crate::ast::statement::Statement; +use crate::ast::NodeId; use crate::diagnostic::Diagnostics; use crate::diagnostic_factories::symbol_not_found; use crate::semantic_analysis::scope::{Scope, ScopeId}; -use crate::semantic_analysis::symbol::{ParameterSymbol, Symbol, SymbolId, VariableSymbol}; +use crate::semantic_analysis::symbol::{Symbol, SymbolId, VariableSymbol}; use std::collections::HashMap; pub struct NameResolutionResult { @@ -175,6 +175,9 @@ fn resolve_names_let_statement( scope .symbols_mut() .insert(let_statement.declared_name_owned(), symbol_id); + // associate the let statement with the symbol id + ctx.nodes_to_symbols + .insert(let_statement.node_id(), symbol_id); } } } diff --git a/dmc-lib/src/semantic_analysis/resolve_types/mod.rs b/dmc-lib/src/semantic_analysis/resolve_types/mod.rs index 19cf64c..1450d7f 100644 --- a/dmc-lib/src/semantic_analysis/resolve_types/mod.rs +++ b/dmc-lib/src/semantic_analysis/resolve_types/mod.rs @@ -108,6 +108,9 @@ fn resolve_types_let_statement(let_statement: &LetStatement, ctx: &mut ResolveTy let symbol_id = ctx.nodes_to_symbols[&let_statement.node_id()]; ctx.symbols_to_type_infos .insert(symbol_id, initializer_type_info_id); + // also save it as the let statement's type info + ctx.nodes_to_type_infos + .insert(let_statement.node_id(), initializer_type_info_id); } fn resolve_types_assign_statement(