From e5c908250cba3d55af0d1bfbd4738b2c12130b21 Mon Sep 17 00:00:00 2001 From: Ayana Alemayehu Date: Thu, 8 May 2025 14:58:55 -0400 Subject: [PATCH 1/7] WIP works with simple sequential and conservative take with parallel block, need to address if statements --- calyx/opt/src/default_passes.rs | 3 +- calyx/opt/src/passes_experimental/cse_exp.rs | 672 +++++++++++++++++++ calyx/opt/src/passes_experimental/mod.rs | 2 + 3 files changed, 676 insertions(+), 1 deletion(-) create mode 100644 calyx/opt/src/passes_experimental/cse_exp.rs diff --git a/calyx/opt/src/default_passes.rs b/calyx/opt/src/default_passes.rs index c6fe054091..d0f8a65c60 100644 --- a/calyx/opt/src/default_passes.rs +++ b/calyx/opt/src/default_passes.rs @@ -14,7 +14,7 @@ use crate::passes::{ }; use crate::passes_experimental::{ CompileSync, CompileSyncWithoutSyncReg, DiscoverExternal, ExternalToRef, - HoleInliner, Metadata, ParToSeq, RegisterUnsharing, + HoleInliner, Metadata, ParToSeq, RegisterUnsharing, CseExp }; use crate::traversal::Named; use crate::{pass_manager::PassManager, register_alias}; @@ -43,6 +43,7 @@ impl PassManager { pm.register_pass::()?; pm.register_pass::()?; pm.register_pass::()?; + pm.register_pass::()?; // Compilation passes pm.register_pass::()?; diff --git a/calyx/opt/src/passes_experimental/cse_exp.rs b/calyx/opt/src/passes_experimental/cse_exp.rs new file mode 100644 index 0000000000..b618d0ed22 --- /dev/null +++ b/calyx/opt/src/passes_experimental/cse_exp.rs @@ -0,0 +1,672 @@ +use std::{cell::RefCell, collections::HashMap}; + +use crate::traversal::{Action, Named, VisResult, Visitor}; +use calyx_ir::{self as ir}; + +const SUPPORTED_STD: &[&str] = &["std_add"]; + +pub struct CseExp { + available_expressions: AvailableExpressions, +} + +impl Named for CseExp { + fn name() -> &'static str { + "cse-exp" + } + + fn description() -> &'static str { + "replace common subexpression uses with already computed values when possible" + } +} + +impl Default for CseExp { + fn default() -> Self { + CseExp { + available_expressions: AvailableExpressions { + current_depth: -1, + safe_depth: -1, + running_expressions: HashMap::< + i32, + HashMap, + >::new(), + per_group_expressions: HashMap::< + String, + HashMap, + >::new(), + }, + } + } +} + +struct ExpressionMetadata { + reg_port: ir::RRC, // id of reg to grab expression from + group: String, // group name that created the expression +} + +struct AvailableExpressions { + current_depth: i32, + safe_depth: i32, + running_expressions: HashMap>, + per_group_expressions: HashMap>, +} + +impl AvailableExpressions { + // stringifys value of cell prototype + pub fn get_val(port: &ir::Port) -> String { + match port.cell_parent().borrow().prototype { + ir::CellType::Constant { val, .. } => return val.to_string(), + ir::CellType::Component { name } => return name.to_string(), + ir::CellType::Primitive { .. } => { + let port_prefix = port.cell_parent().borrow().name(); + let port_suffix = port.name; + return format!("{port_prefix}{port_suffix}"); + } + ir::CellType::ThisComponent => { + return "absolutely no idea".to_string(); + } + } + } + /* + increment depth and potentially safe depth, allocating + a new HashMap for this depth's expressions + */ + fn inc_depth(&mut self, safe_depth: bool) -> () { + // invariant check + assert!( + self.safe_depth <= self.current_depth, + "safe depth somehow exceeded current depth" + ); + // only increment if current_depth is on par with safe depth, + // otherwise either safe_depth is false OR we are in an unsafe + // zone. + if safe_depth && self.current_depth == self.safe_depth { + self.current_depth += 1; + self.safe_depth += 1; + } else { + self.current_depth += 1; + } + // this key should never exist already + let dbg_depth = self.current_depth; + if self.running_expressions.contains_key(&self.current_depth) { + panic!( + "running expressions somehow already contains current depth {dbg_depth} key" + ); + } + let current_depth_expressions: HashMap = + HashMap::::new(); + self.running_expressions + .insert(self.current_depth, current_depth_expressions); + log::debug!( + "incremented current depth to {dbg_depth} and allocated hashmap for expressions" + ); + } + /* + decrement depth and potentially safe depth, deleting the HashMap + allocated for this depth's expressions + */ + fn dec_depth(&mut self) -> () { + // invariant check + let dbg_deleted_depth = self.current_depth; + assert!( + self.safe_depth <= self.current_depth, + "safe depth somehow exceeded current depth" + ); + if self.current_depth == self.safe_depth { + self.safe_depth -= 1; + self.current_depth -= 1; + } else { + self.current_depth -= 1; + } + let dbg_depth = self.current_depth; + self.running_expressions.remove(&self.current_depth); + log::debug!( + "decremented current depth to {dbg_depth} and removed hashmap for expressions at depth {dbg_deleted_depth}" + ); + } + + /* + add to current_depth's running_expressions available subexpressions from + supported operations + */ + fn add_exp( + &mut self, + assignments: &Vec>, // a specific group's assignments + group: String, // the group with the assignments in question + ) -> () { + let mut intermediate_exp: HashMap = + HashMap::::new(); + let mut completed_exp = HashMap::::new(); + for assign in assignments.iter() { + // early breakouts + if assign.dst.borrow().is_hole() { + continue; + } + let operation = + match assign.dst.borrow().cell_parent().borrow().type_name() { + Some(v) => v, + None => continue, + }; + if !(SUPPORTED_STD.contains(&operation.to_string().as_str())) { + // here we check if a register is latching an existing subexpression + let dst_port_name = assign.dst.borrow().name; + if operation.to_string().as_str() == "std_reg" + && dst_port_name.to_string().as_str() == "in" + { + let latching_cadidate = + assign.src.borrow().cell_parent().borrow().name(); + let src_port_name = assign.src.borrow().name; + log::debug!( + "latching candidate {latching_cadidate}.{src_port_name}" + ); + if completed_exp + .contains_key(&latching_cadidate.to_string()) + && self.current_depth <= self.safe_depth + && src_port_name.to_string().as_str() == "out" + { + let new_exp = ExpressionMetadata { + reg_port: assign + .dst + .borrow() + .cell_parent() + .borrow() + .get("out"), // <------ right now only the in port is written down, we need it to be the .out port. TODO + group: group.clone(), + }; + match self + .running_expressions + .get_mut(&self.current_depth) + { + Some(current_depth_expressions) => { + match completed_exp + .get(&latching_cadidate.to_string()) + { + Some(string_expression) => { + let dbg_parent = new_exp + .reg_port + .borrow() + .cell_parent() + .borrow() + .name(); + let dbg_port = + new_exp.reg_port.borrow().name; + let dbg_depth = self.current_depth; + log::debug!( + "[GEN] adding expression {string_expression} with parent port {dbg_parent}.{dbg_port} to running expressions at depth {dbg_depth}" + ); + current_depth_expressions.insert( + string_expression.to_string(), + new_exp, + ); + } + None => { + panic!( + "string expression not found in current depth expressions" + ); + } + } + } + None => { + panic!( + "current depth not found in running expressions" + ); + } + } + } + } + // TODO: ensure expresion is latched and safe_depth is >= cur_depth before adding to avaialble expresions + continue; + } + // check intermediate_exp if already contains expression + let operation_cell_name = + assign.dst.borrow().cell_parent().borrow().name(); + if !intermediate_exp.contains_key(&operation_cell_name.to_string()) + { + intermediate_exp.insert( + operation_cell_name.to_string(), + AvailableExpressions::get_val(&assign.src.borrow()), + ); + continue; + } + // else we have completed this subexpression + else { + let dest = + intermediate_exp.get(&operation_cell_name.to_string()); + match dest { + Some(destination) => { + // grab full subexpression + let cdepth = self.current_depth; + let source = + AvailableExpressions::get_val(&assign.src.borrow()); + let expression = + format!("{source}({operation}){destination}"); + log::debug!( + "added {expression} for depth {cdepth} to completed (intermediate) expressions" + ); + completed_exp.insert( + operation_cell_name.to_string(), + expression, + ); + } + None => { + panic!("missing key?"); + } + } + } + } + } + + /* + identify destroyed expressions from register overwrites + and remove from all depths. + */ + fn kill_exp( + &mut self, + assignments: &Vec>, + group: String, + ) { + let mut remove_expressions: Vec = Vec::new(); + for assign in assignments.iter() { + if assign.dst.borrow().is_hole() { + continue; + } + let operation = + match assign.dst.borrow().cell_parent().borrow().type_name() { + Some(v) => v, + None => continue, + }; + // we need to see if a register that is containing a currently latched + // subexpression is being overwritted + let dst_port = assign.dst.borrow(); + if operation.to_string().as_str() == "std_reg" + && dst_port.name.to_string().as_str() == "in" + { + for depth in 0..(self.current_depth + 1) { + let e = self.running_expressions.get_mut(&depth); + match e { + Some(expressions) => { + for (string_expression, metadata) in + expressions.into_iter() + { + if metadata.group != group + && metadata + .reg_port + .borrow() + .cell_parent() + .borrow() + .name() + == dst_port + .cell_parent() + .borrow() + .name() + { + remove_expressions + .push(string_expression.to_string()); + } + } + } + None => { + panic!("no HashMap allocated for depth {depth}?"); + } + } + } + } + for killed_expression in remove_expressions.iter() { + for depth in 0..(self.current_depth + 1) { + let e = self.running_expressions.get_mut(&depth); + match e { + Some(expressions) => { + if expressions.remove(killed_expression).is_some() { + log::debug!( + "[KILL] removed expression {killed_expression} from available expressions at depth {depth}" + ); + } + } + None => { + panic!("no HashMap allocated for depth {depth}?"); + } + } + } + } + } + } + + /* + Do one of two things: + 1) if group not in self.group_expressions, do + self.group_expressions[group] = self.running_expressions + 2) else, do self.group_expressions[group] ∩ self.running_expressions + */ + fn group_exp(&mut self, group: String) { + if self.per_group_expressions.contains_key(&group) { + // do 2) + let mut remove_expressions: Vec = Vec::new(); + let g_e = self.per_group_expressions.get_mut(&group); + match g_e { + Some(group_expressions) => { + for (group_string_expression, _) in &mut *group_expressions + { + let mut remove_flag = true; + for depth in 0..(self.current_depth + 1) { + let e = self.running_expressions.get_mut(&depth); + match e { + Some(expressions) => { + if expressions + .contains_key(group_string_expression) + { + remove_flag = false; + } + } + None => { + panic!( + "no HashMap allocated for depth {depth}?" + ); + } + } + } + if remove_flag { + remove_expressions + .push(group_string_expression.clone()); + } + } + for removed_expression in remove_expressions.iter() { + if group_expressions + .remove(removed_expression) + .is_some() + { + log::debug!( + "[GROUP-KILL] removed expression {removed_expression} from availalbe expressions for group {group}" + ); + } else { + panic!( + "expected expression to exist in group expressions" + ); + } + } + } + None => { + panic!("expected group expressions to exist"); + } + } + } else { + // do 1) + let mut new_group_expressions: HashMap = + HashMap::::new(); + for depth in 0..(self.current_depth + 1) { + let e = self.running_expressions.get_mut(&depth); + match e { + Some(expressions) => { + for (string_expression, metadata) in + expressions.into_iter() + { + new_group_expressions.insert( + string_expression.clone(), + ExpressionMetadata { + reg_port: metadata.reg_port.clone(), + group: metadata.group.clone(), + }, + ); + } + } + None => { + panic!("no HashMap allocated for depth {depth}?"); + } + } + } + let dbg_depth = self.current_depth; + log::debug!( + "[GROUP-GEN] inserted all running expressions from depth {dbg_depth} downwards for group {group}" + ); + self.per_group_expressions + .insert(group, new_group_expressions); + } + } + /* + in-place mutate a group given its availalbe expressions by doing + the following for supported operations: + 1) identify subexpressions created and used within the group + 2) figure out which of those subexpressions have already been + saved in per_group expressions + 3) replace all "=(redundant_operation).out" with latched register + outs + */ + fn optimize( + &mut self, + group_obj: &mut std::cell::RefMut, + group: String, // the group with the assignments in question + ) -> () { + let mut intermediate_exp: HashMap = + HashMap::::new(); + let mut completed_exp = HashMap::::new(); + let mut cse_rewriter: ir::rewriter::PortRewriteMap = + ir::rewriter::PortRewriteMap::new(); + let assignments = &mut group_obj.assignments; + for assign in &mut assignments.iter_mut() { + // early breakouts + if assign.dst.borrow().is_hole() { + continue; + } + let operation = + match assign.dst.borrow().cell_parent().borrow().type_name() { + Some(v) => v, + None => continue, + }; + if !(SUPPORTED_STD.contains(&operation.to_string().as_str())) { + // here we check if an operation is reading from a redundant operation + let cse_candidate_operation = match assign + .src + .borrow() + .cell_parent() + .borrow() + .type_name() + { + Some(v) => v, + None => continue, + }; + if !(SUPPORTED_STD + .contains(&cse_candidate_operation.to_string().as_str())) + { + continue; + } + if assign.src.borrow().name != "out" { + continue; + } + // at this point we are confident that its a supported operation and a .out port + // being read by some other cell. check if it contains a subexpression thats already computed + let supported_operation_key = assign + .src + .borrow() + .cell_parent() + .borrow() + .name() + .to_string(); + let string_expression = + match completed_exp.get(&supported_operation_key) { + Some(v) => v, + None => continue, + }; + let current_group_subexpressions = match self + .per_group_expressions + .get(&group) + { + Some(v) => v, + None => { + panic!( + "group should have available expressions at this point" + ) + } + }; + match current_group_subexpressions.get(string_expression) { + Some(potential_common_subexpression) => { + if potential_common_subexpression.group != group { + log::debug!( + "common subexpression {string_expression} identified in {group}" + ); + /* + i think now you add a mapping from redun_calculation.out to latched_exp_reg.out + aka mapping from assignment src to cse port + */ + let dbg_canonical_src = + assign.src.borrow().canonical(); + let dbg_canonical_val = + potential_common_subexpression + .reg_port + .clone() + .borrow() + .canonical(); + log::debug!( + "[OPTIMIZE] applying mapping from {dbg_canonical_src} -> {dbg_canonical_val} for group {group}" + ); + cse_rewriter.insert( + assign.src.clone().borrow().canonical(), + potential_common_subexpression.reg_port.clone(), + ); + let rewriter = ir::Rewriter { + port_map: cse_rewriter.clone(), + ..Default::default() + }; + let mut asgn = assign.clone(); + rewriter.rewrite_assign(&mut asgn); + *assign = asgn; + } + } + None => continue, + } + continue; + } + // check intermediate_exp if already contains expression + let operation_cell_name = + assign.dst.borrow().cell_parent().borrow().name(); + if !intermediate_exp.contains_key(&operation_cell_name.to_string()) + { + intermediate_exp.insert( + operation_cell_name.to_string(), + AvailableExpressions::get_val(&assign.src.borrow()), + ); + continue; + } + // else we have completed this subexpression + else { + let dest = + intermediate_exp.get(&operation_cell_name.to_string()); + match dest { + Some(destination) => { + // grab full subexpression + let cdepth = self.current_depth; + let source = + AvailableExpressions::get_val(&assign.src.borrow()); + let expression = + format!("{source}({operation}){destination}"); + log::debug!( + "added {expression} for depth {cdepth} to completed (intermediate) expressions" + ); + completed_exp.insert( + operation_cell_name.to_string(), + expression, + ); + } + None => { + panic!("missing key?"); + } + } + } + } + // let rewriter = ir::Rewriter { + // port_map: cse_rewriter, + // ..Default::default() + // }; + // log::debug!("rewriter invoked for group {group}"); + // let mut asgns = group_obj.assignments.clone(); + // for assign in asgns.iter_mut() { + // rewriter.rewrite_assign(assign); + // } + // group_obj.assignments = asgns; + // rewriter.rewrite_control(&mut comp.control.borrow_mut()); + } +} + +impl Visitor for CseExp { + /* + Start is treated like a seq block, so simple safe increment + of depth + */ + fn start( + &mut self, + _comp: &mut ir::Component, + _sigs: &ir::LibrarySignatures, + _comps: &[ir::Component], + ) -> VisResult { + log::debug!("toplevel start"); + // create depth 0 dictionary, this is basically a seq block + self.available_expressions.inc_depth(true); + Ok(Action::Continue) + } + fn start_seq( + &mut self, + _s: &mut calyx_ir::Seq, + _comp: &mut calyx_ir::Component, + _sigs: &calyx_ir::LibrarySignatures, + _comps: &[calyx_ir::Component], + ) -> VisResult { + log::debug!("start_seq"); + self.available_expressions.inc_depth(true); + Ok(Action::Continue) + } + fn finish_seq( + &mut self, + _s: &mut calyx_ir::Seq, + _comp: &mut calyx_ir::Component, + _sigs: &calyx_ir::LibrarySignatures, + _comps: &[calyx_ir::Component], + ) -> VisResult { + log::debug!("finish_seq"); + self.available_expressions.dec_depth(); + Ok(Action::Continue) + } + /* + Do: + 1) add expressions this group creates + 2) remove expressions this group killed + 3) update the expressions availalbe to this group specifically + which is either... + 3.0) adding the current running expressions entirely if + there arent expressions logged for the group already + 3.1) adding the intersection of current running expressions + /w this groups logged expressions + */ + fn enable( + &mut self, + _s: &mut calyx_ir::Enable, + _comp: &mut calyx_ir::Component, + _sigs: &calyx_ir::LibrarySignatures, + _comps: &[calyx_ir::Component], + ) -> VisResult { + let group = &_s.group; + let group_name = group.borrow().name().to_string(); + log::debug!("group {group_name} enable"); + self.available_expressions + .add_exp(&group.borrow().assignments, group_name.clone()); + self.available_expressions + .kill_exp(&group.borrow().assignments, group_name.clone()); + self.available_expressions.group_exp(group_name.clone()); + Ok(Action::Continue) + } + + /* + Remove the identified redundant common subexpressions in each group + */ + fn finish( + &mut self, + _comp: &mut calyx_ir::Component, + _sigs: &calyx_ir::LibrarySignatures, + _comps: &[calyx_ir::Component], + ) -> VisResult { + log::debug!("optimize"); + for group in _comp.get_groups_mut().iter_mut() { + let group_name = group.borrow().name().to_string(); + log::debug!("group {group_name}"); + self.available_expressions + .optimize(&mut group.borrow_mut(), group_name); + } + Ok(Action::Continue) + } +} diff --git a/calyx/opt/src/passes_experimental/mod.rs b/calyx/opt/src/passes_experimental/mod.rs index 9edc16cc27..e7d04088c8 100644 --- a/calyx/opt/src/passes_experimental/mod.rs +++ b/calyx/opt/src/passes_experimental/mod.rs @@ -1,3 +1,4 @@ +mod cse_exp; mod discover_external; mod external_to_ref; mod hole_inliner; @@ -6,6 +7,7 @@ mod par_to_seq; mod register_unsharing; mod sync; +pub use cse_exp::CseExp; pub use discover_external::DiscoverExternal; pub use external_to_ref::ExternalToRef; pub use hole_inliner::HoleInliner; From 213d5d40bcb34253468c834b188f7f0e10061820 Mon Sep 17 00:00:00 2001 From: Ayana Alemayehu Date: Sat, 10 May 2025 16:18:16 -0400 Subject: [PATCH 2/7] fixed dec_depth not deleting right key and also made my optimization support duplicates with first in first used (unless killed) semantics --- calyx/opt/src/passes_experimental/cse_exp.rs | 426 +++++++++++++------ 1 file changed, 293 insertions(+), 133 deletions(-) diff --git a/calyx/opt/src/passes_experimental/cse_exp.rs b/calyx/opt/src/passes_experimental/cse_exp.rs index b618d0ed22..153ba27c6f 100644 --- a/calyx/opt/src/passes_experimental/cse_exp.rs +++ b/calyx/opt/src/passes_experimental/cse_exp.rs @@ -27,11 +27,11 @@ impl Default for CseExp { safe_depth: -1, running_expressions: HashMap::< i32, - HashMap, + HashMap>, >::new(), per_group_expressions: HashMap::< String, - HashMap, + HashMap>, >::new(), }, } @@ -46,8 +46,9 @@ struct ExpressionMetadata { struct AvailableExpressions { current_depth: i32, safe_depth: i32, - running_expressions: HashMap>, - per_group_expressions: HashMap>, + running_expressions: HashMap>>, // its a vector to deal with duplicates + per_group_expressions: + HashMap>>, } impl AvailableExpressions { @@ -66,6 +67,35 @@ impl AvailableExpressions { } } } + + fn contains_metadata( + &self, + string_expression: &String, + metadata: &ExpressionMetadata, + ) -> bool { + let mut contains_flag = false; + for depth in 0..(self.current_depth + 1) { + match self.running_expressions.get(&depth) { + Some(expressions) => match expressions.get(string_expression) { + Some(metadata_vec) => { + for met in metadata_vec { + if met.group == metadata.group + && met.reg_port == metadata.reg_port + { + contains_flag = true; + } + } + } + None => {} + }, + None => { + panic!("no HashMap allocated for depth {depth}?"); + } + } + } + return contains_flag; + } + /* increment depth and potentially safe depth, allocating a new HashMap for this depth's expressions @@ -88,12 +118,19 @@ impl AvailableExpressions { // this key should never exist already let dbg_depth = self.current_depth; if self.running_expressions.contains_key(&self.current_depth) { + let dbg_map_len = + match self.running_expressions.get(&self.current_depth) { + Some(v) => v.keys().len(), + None => panic! {"??"}, + }; panic!( - "running expressions somehow already contains current depth {dbg_depth} key" + "running expressions somehow already contains current depth key {dbg_depth} len {dbg_map_len}" ); } - let current_depth_expressions: HashMap = - HashMap::::new(); + let current_depth_expressions: HashMap< + String, + Vec, + > = HashMap::>::new(); self.running_expressions .insert(self.current_depth, current_depth_expressions); log::debug!( @@ -105,8 +142,8 @@ impl AvailableExpressions { allocated for this depth's expressions */ fn dec_depth(&mut self) -> () { + let deleted_depth = self.current_depth; // invariant check - let dbg_deleted_depth = self.current_depth; assert!( self.safe_depth <= self.current_depth, "safe depth somehow exceeded current depth" @@ -118,10 +155,11 @@ impl AvailableExpressions { self.current_depth -= 1; } let dbg_depth = self.current_depth; - self.running_expressions.remove(&self.current_depth); - log::debug!( - "decremented current depth to {dbg_depth} and removed hashmap for expressions at depth {dbg_deleted_depth}" - ); + if self.running_expressions.remove(&deleted_depth).is_some() { + log::debug!( + "decremented current depth to {dbg_depth} and removed hashmap for expressions at depth {deleted_depth}" + ); + } } /* @@ -163,6 +201,20 @@ impl AvailableExpressions { && self.current_depth <= self.safe_depth && src_port_name.to_string().as_str() == "out" { + let string_expression = match completed_exp + .get(&latching_cadidate.to_string()) + { + Some(v) => v, + None => panic!( + "expected completed expressions to contain latching candidaate string" + ), + }; + // if self.contains_exp(string_expression) { + // log::debug!( + // "already contain {string_expression} in running expressions" + // ); + // continue; + // } let new_exp = ExpressionMetadata { reg_port: assign .dst @@ -177,30 +229,33 @@ impl AvailableExpressions { .get_mut(&self.current_depth) { Some(current_depth_expressions) => { - match completed_exp - .get(&latching_cadidate.to_string()) + let dbg_depth: i32 = self.current_depth; + let dbg_parent = new_exp + .reg_port + .borrow() + .cell_parent() + .borrow() + .name(); + let dbg_port = new_exp.reg_port.borrow().name; + match current_depth_expressions + .get_mut(string_expression) { - Some(string_expression) => { - let dbg_parent = new_exp - .reg_port - .borrow() - .cell_parent() - .borrow() - .name(); - let dbg_port = - new_exp.reg_port.borrow().name; - let dbg_depth = self.current_depth; + Some(string_expression_sources) => { + // existing list of subexpressions will have another source appended on it log::debug!( - "[GEN] adding expression {string_expression} with parent port {dbg_parent}.{dbg_port} to running expressions at depth {dbg_depth}" - ); - current_depth_expressions.insert( - string_expression.to_string(), - new_exp, + "[GEN] adding {string_expression} with parent port {dbg_parent}.{dbg_port} to existing list at depth {dbg_depth}" ); + string_expression_sources.push(new_exp); } None => { - panic!( - "string expression not found in current depth expressions" + // new list of subexpressions will be allocated + let new_exp_sources = vec![new_exp]; + log::debug!( + "[GEN] adding expression {string_expression} with parent port {dbg_parent}.{dbg_port} to new list of running expressions at depth {dbg_depth}" + ); + current_depth_expressions.insert( + string_expression.to_string(), + new_exp_sources, ); } } @@ -264,7 +319,7 @@ impl AvailableExpressions { assignments: &Vec>, group: String, ) { - let mut remove_expressions: Vec = Vec::new(); + // let mut remove_expressions: Vec = Vec::new(); for assign in assignments.iter() { if assign.dst.borrow().is_hole() { continue; @@ -281,44 +336,65 @@ impl AvailableExpressions { && dst_port.name.to_string().as_str() == "in" { for depth in 0..(self.current_depth + 1) { - let e = self.running_expressions.get_mut(&depth); + let e = self.running_expressions.get(&depth); + let mut updates = + HashMap::>::new(); match e { Some(expressions) => { - for (string_expression, metadata) in + for (string_expression, metadata_vec) in expressions.into_iter() { - if metadata.group != group - && metadata - .reg_port - .borrow() - .cell_parent() - .borrow() - .name() - == dst_port + let mut new_expression_sources = + Vec::::new(); + for metadata in metadata_vec.into_iter() { + // either this was introduced in this group, or we don't share a cell_parent + // if the above is true then we keep the expression + if metadata.group == group + || metadata + .reg_port + .borrow() .cell_parent() .borrow() .name() - { - remove_expressions - .push(string_expression.to_string()); + != dst_port + .cell_parent() + .borrow() + .name() + { + new_expression_sources.push( + ExpressionMetadata { + reg_port: metadata + .reg_port + .clone(), + group: metadata.group.clone(), + }, + ); + } else { + let dbg_parent = dst_port + .cell_parent() + .borrow() + .name(); + let dbg_port = + metadata.reg_port.borrow().name; + log::debug!( + "[KILL] removed {string_expression} with parent port {dbg_parent}.{dbg_port} from expressions at depth {depth}" + ); + } } + updates.insert( + string_expression.clone(), + new_expression_sources, + ); } } None => { panic!("no HashMap allocated for depth {depth}?"); } } - } - } - for killed_expression in remove_expressions.iter() { - for depth in 0..(self.current_depth + 1) { - let e = self.running_expressions.get_mut(&depth); - match e { + match self.running_expressions.get_mut(&depth) { Some(expressions) => { - if expressions.remove(killed_expression).is_some() { - log::debug!( - "[KILL] removed expression {killed_expression} from available expressions at depth {depth}" - ); + for (key, value) in updates { + expressions.insert(key, value); } } None => { @@ -327,6 +403,29 @@ impl AvailableExpressions { } } } + // for killed_expression in remove_expressions.iter() { + // for depth in 0..(self.current_depth + 1) { + // match self.running_expressions.get_mut(&depth) { + // Some(expressions) => { + // match expressions.get(killed_expression) { + // Some(expression_sources) => { + // // there is a vector of expressions available + // let new_expression_sources = + // Vec::::new(); + // } + // } + // if expressions.remove(killed_expression).is_some() { + // log::debug!( + // "[KILL] removed expression {killed_expression} from available expressions at depth {depth}" + // ); + // } + // } + // None => { + // panic!("no HashMap allocated for depth {depth}?"); + // } + // } + // } + // } } } @@ -339,72 +438,120 @@ impl AvailableExpressions { fn group_exp(&mut self, group: String) { if self.per_group_expressions.contains_key(&group) { // do 2) - let mut remove_expressions: Vec = Vec::new(); - let g_e = self.per_group_expressions.get_mut(&group); + let mut new_group_expressions = + HashMap::>::new(); + // let mut remove_expressions: Vec = Vec::new(); + let g_e = self.per_group_expressions.get(&group); match g_e { Some(group_expressions) => { - for (group_string_expression, _) in &mut *group_expressions + for (group_string_expression, metadata_vec) in + group_expressions { - let mut remove_flag = true; - for depth in 0..(self.current_depth + 1) { - let e = self.running_expressions.get_mut(&depth); - match e { - Some(expressions) => { - if expressions - .contains_key(group_string_expression) - { - remove_flag = false; - } - } - None => { - panic!( - "no HashMap allocated for depth {depth}?" - ); - } + let mut new_group_expression_vec = + Vec::::new(); + for metadata in metadata_vec.into_iter() { + if self.contains_metadata( + group_string_expression, + metadata, + ) { + new_group_expression_vec.push( + ExpressionMetadata { + group: metadata.group.clone(), + reg_port: metadata.reg_port.clone(), + }, + ) + } else { + let dbg_parent = metadata + .reg_port + .borrow() + .cell_parent() + .borrow() + .name(); + let dbg_port = metadata.reg_port.borrow().name; + log::debug!( + "[GROUP-KILL] removed expression {group_string_expression} with parent port {dbg_parent}.{dbg_port}" + ); } } - if remove_flag { - remove_expressions - .push(group_string_expression.clone()); - } - } - for removed_expression in remove_expressions.iter() { - if group_expressions - .remove(removed_expression) - .is_some() - { - log::debug!( - "[GROUP-KILL] removed expression {removed_expression} from availalbe expressions for group {group}" - ); - } else { - panic!( - "expected expression to exist in group expressions" - ); - } + new_group_expressions.insert( + group_string_expression.clone(), + new_group_expression_vec, + ); } } None => { panic!("expected group expressions to exist"); } } + self.per_group_expressions + .insert(group.clone(), new_group_expressions); + // let modify_g_e = self.per_group_expressions.get_mut(&group); + // match modify_g_e { + // Some(group_expressions) => { + // for removed_expression in remove_expressions.iter() { + // if group_expressions + // .remove(removed_expression) + // .is_some() + // { + // log::debug!( + // "[GROUP-KILL] removed expression {removed_expression} from availalbe expressions for group {group}" + // ); + // } else { + // panic!( + // "expected expression to exist in group expressions" + // ); + // } + // } + // } + // None => { + // panic!("expected group expressions to exist"); + // } + // } } else { // do 1) - let mut new_group_expressions: HashMap = - HashMap::::new(); + let mut new_group_expressions: HashMap< + String, + Vec, + > = HashMap::>::new(); for depth in 0..(self.current_depth + 1) { let e = self.running_expressions.get_mut(&depth); match e { Some(expressions) => { - for (string_expression, metadata) in + for (string_expression, metadata_vec) in expressions.into_iter() { - new_group_expressions.insert( - string_expression.clone(), - ExpressionMetadata { - reg_port: metadata.reg_port.clone(), - group: metadata.group.clone(), - }, - ); + for metadata in metadata_vec.into_iter() { + match new_group_expressions + .get_mut(string_expression) + { + Some(group_expression_vec) => { + group_expression_vec.push( + ExpressionMetadata { + reg_port: metadata + .reg_port + .clone(), + group: metadata.group.clone(), + }, + ) + } + None => { + let mut new_group_expression_vec = + Vec::::new(); + new_group_expression_vec.push( + ExpressionMetadata { + reg_port: metadata + .reg_port + .clone(), + group: metadata.group.clone(), + }, + ); + new_group_expressions.insert( + string_expression.clone(), + new_group_expression_vec, + ); + } + } + } } } None => { @@ -496,37 +643,50 @@ impl AvailableExpressions { } }; match current_group_subexpressions.get(string_expression) { - Some(potential_common_subexpression) => { - if potential_common_subexpression.group != group { - log::debug!( - "common subexpression {string_expression} identified in {group}" - ); - /* - i think now you add a mapping from redun_calculation.out to latched_exp_reg.out - aka mapping from assignment src to cse port - */ - let dbg_canonical_src = - assign.src.borrow().canonical(); - let dbg_canonical_val = - potential_common_subexpression - .reg_port - .clone() - .borrow() - .canonical(); - log::debug!( - "[OPTIMIZE] applying mapping from {dbg_canonical_src} -> {dbg_canonical_val} for group {group}" - ); - cse_rewriter.insert( - assign.src.clone().borrow().canonical(), - potential_common_subexpression.reg_port.clone(), - ); - let rewriter = ir::Rewriter { - port_map: cse_rewriter.clone(), - ..Default::default() - }; - let mut asgn = assign.clone(); - rewriter.rewrite_assign(&mut asgn); - *assign = asgn; + Some(potential_common_subexpression_vec) => { + if potential_common_subexpression_vec.len() > 0 { + // getting the 0th index will get the earliest detected common subexpression + let potential_common_subexpression = + potential_common_subexpression_vec + .get(0) + .expect(&format!( + "expected zero index expression" + )); + if potential_common_subexpression.group != group { + log::debug!( + "common subexpression {string_expression} identified in {group}" + ); + /* + i think now you add a mapping from redun_calculation.out to latched_exp_reg.out + aka mapping from assignment src to cse port + */ + let dbg_canonical_src = + assign.src.borrow().canonical(); + let dbg_canonical_val = + potential_common_subexpression + .reg_port + .clone() + .borrow() + .canonical(); + log::debug!( + "[OPTIMIZE] applying mapping from {dbg_canonical_src} -> {dbg_canonical_val} for group {group}" + ); + cse_rewriter.insert( + assign.src.clone().borrow().canonical(), + potential_common_subexpression + .reg_port + .clone(), + ); + let rewriter = ir::Rewriter { + port_map: cse_rewriter.clone(), + ..Default::default() + }; + let mut asgn = assign.clone(); + rewriter.rewrite_assign(&mut asgn); + *assign = asgn; + } + } else { + continue; } } None => continue, From 257bee36a80f3ac764c1700d6113f50bb30d194e Mon Sep 17 00:00:00 2001 From: Ayana Alemayehu Date: Sat, 10 May 2025 16:24:30 -0400 Subject: [PATCH 3/7] removed comments --- calyx/opt/src/passes_experimental/cse_exp.rs | 62 -------------------- 1 file changed, 62 deletions(-) diff --git a/calyx/opt/src/passes_experimental/cse_exp.rs b/calyx/opt/src/passes_experimental/cse_exp.rs index 153ba27c6f..70def28fb6 100644 --- a/calyx/opt/src/passes_experimental/cse_exp.rs +++ b/calyx/opt/src/passes_experimental/cse_exp.rs @@ -209,12 +209,6 @@ impl AvailableExpressions { "expected completed expressions to contain latching candidaate string" ), }; - // if self.contains_exp(string_expression) { - // log::debug!( - // "already contain {string_expression} in running expressions" - // ); - // continue; - // } let new_exp = ExpressionMetadata { reg_port: assign .dst @@ -403,29 +397,6 @@ impl AvailableExpressions { } } } - // for killed_expression in remove_expressions.iter() { - // for depth in 0..(self.current_depth + 1) { - // match self.running_expressions.get_mut(&depth) { - // Some(expressions) => { - // match expressions.get(killed_expression) { - // Some(expression_sources) => { - // // there is a vector of expressions available - // let new_expression_sources = - // Vec::::new(); - // } - // } - // if expressions.remove(killed_expression).is_some() { - // log::debug!( - // "[KILL] removed expression {killed_expression} from available expressions at depth {depth}" - // ); - // } - // } - // None => { - // panic!("no HashMap allocated for depth {depth}?"); - // } - // } - // } - // } } } @@ -485,28 +456,6 @@ impl AvailableExpressions { } self.per_group_expressions .insert(group.clone(), new_group_expressions); - // let modify_g_e = self.per_group_expressions.get_mut(&group); - // match modify_g_e { - // Some(group_expressions) => { - // for removed_expression in remove_expressions.iter() { - // if group_expressions - // .remove(removed_expression) - // .is_some() - // { - // log::debug!( - // "[GROUP-KILL] removed expression {removed_expression} from availalbe expressions for group {group}" - // ); - // } else { - // panic!( - // "expected expression to exist in group expressions" - // ); - // } - // } - // } - // None => { - // panic!("expected group expressions to exist"); - // } - // } } else { // do 1) let mut new_group_expressions: HashMap< @@ -730,17 +679,6 @@ impl AvailableExpressions { } } } - // let rewriter = ir::Rewriter { - // port_map: cse_rewriter, - // ..Default::default() - // }; - // log::debug!("rewriter invoked for group {group}"); - // let mut asgns = group_obj.assignments.clone(); - // for assign in asgns.iter_mut() { - // rewriter.rewrite_assign(assign); - // } - // group_obj.assignments = asgns; - // rewriter.rewrite_control(&mut comp.control.borrow_mut()); } } From 7362893fed0bad8db2cb28fbea75eacc04c6cefb Mon Sep 17 00:00:00 2001 From: Ayana Alemayehu Date: Mon, 12 May 2025 17:29:43 -0400 Subject: [PATCH 4/7] stopped tracking depth, will refactor soon. made own visitor to control traversals and tested it with if branches. Seems to work. Refactor coming next --- calyx/opt/src/passes_experimental/cse_exp.rs | 397 +++++++++++++++++-- 1 file changed, 354 insertions(+), 43 deletions(-) diff --git a/calyx/opt/src/passes_experimental/cse_exp.rs b/calyx/opt/src/passes_experimental/cse_exp.rs index 70def28fb6..b65fafd924 100644 --- a/calyx/opt/src/passes_experimental/cse_exp.rs +++ b/calyx/opt/src/passes_experimental/cse_exp.rs @@ -1,7 +1,7 @@ -use std::{cell::RefCell, collections::HashMap}; - use crate::traversal::{Action, Named, VisResult, Visitor}; use calyx_ir::{self as ir}; +use ir::Control::{self as Control}; +use std::{collections::HashMap, rc::Rc, string}; const SUPPORTED_STD: &[&str] = &["std_add"]; @@ -43,6 +43,15 @@ struct ExpressionMetadata { group: String, // group name that created the expression } +impl Clone for ExpressionMetadata { + fn clone(&self) -> Self { + ExpressionMetadata { + reg_port: self.reg_port.clone(), + group: self.group.clone(), + } + } +} + struct AvailableExpressions { current_depth: i32, safe_depth: i32, @@ -68,6 +77,94 @@ impl AvailableExpressions { } } + pub fn intersection( + one: &AvailableExpressions, + two: &AvailableExpressions, + ) -> ( + HashMap>, + HashMap>>, + ) { + // <- i was working on an intersection function to take two available expressions structs/objects and create a new hashmap representing their intersections (to then finish the correct if implementation) + // we follow one's order of stuff for the intersection, since it has to exist in both branches its an alright order to follow + let mut running_intersection = + HashMap::>::new(); + let mut group_intersection = + HashMap::>>::new(); + for (_, expressions) in one.running_expressions.iter() { + for (string_expression, metadata_vec) in expressions { + let mut new_metadata_vec = Vec::::new(); + for metadata in metadata_vec { + if two.contains_metadata(&string_expression, &metadata) { + new_metadata_vec.push(metadata.clone()); + } + } + if new_metadata_vec.len() > 0 { + running_intersection + .insert(string_expression.clone(), new_metadata_vec); + } + } + } + for (group, expressions) in one.per_group_expressions.iter() { + // do intersection if both available expressions have the group + if two.per_group_expressions.contains_key(group) { + for (string_expression, metadata_vec) in expressions { + for metadata in metadata_vec { + if two.group_contains_metadata( + &group, + &string_expression, + &metadata, + ) { + if !group_intersection.contains_key(group) { + let group_expressions = HashMap::< + String, + Vec, + >::new( + ); + group_intersection + .insert(group.clone(), group_expressions); + } + let group_expressions = + group_intersection.get_mut(group).expect( + &format!("expected expressions {group}"), + ); + if group_expressions.contains_key(string_expression) + { + let mut group_metadata_vec = group_expressions.get(string_expression).expect(&format!("expected metadata vec {string_expression}")).clone(); + group_metadata_vec.push(metadata.clone()); + group_expressions + .insert(group.clone(), group_metadata_vec); + } else { + let mut group_metadata_vec = + Vec::::new(); + group_metadata_vec.push(metadata.clone()); + group_expressions + .insert(group.clone(), group_metadata_vec); + } + } + } + } + } else { + // else do union since the group was unique to the one branch + group_intersection.insert(group.clone(), expressions.clone()); + } + } + // catch groups that two saw that one didn't see + for (group, expressions) in two.per_group_expressions.iter() { + if !one.per_group_expressions.contains_key(group) { + group_intersection.insert(group.clone(), expressions.clone()); + } + } + return (running_intersection, group_intersection); + } + fn clone(&self) -> AvailableExpressions { + AvailableExpressions { + current_depth: self.current_depth, + safe_depth: self.safe_depth, + running_expressions: self.running_expressions.clone(), + per_group_expressions: self.per_group_expressions.clone(), + } + } + // for checking running_expressions fn contains_metadata( &self, string_expression: &String, @@ -96,6 +193,36 @@ impl AvailableExpressions { return contains_flag; } + // for checking group_expressions + fn group_contains_metadata( + &self, + group: &String, + string_expression: &String, + metadata: &ExpressionMetadata, + ) -> bool { + let mut contains_flag = false; + match self.per_group_expressions.get(group) { + Some(expressions) => match expressions.get(string_expression) { + Some(metadata_vec) => { + for met in metadata_vec { + if met.group == metadata.group + && met.reg_port == metadata.reg_port + { + contains_flag = true; + } + } + } + None => {} + }, + None => { + log::debug!( + "function called without containing hashmap for group {group}" + ); + } + } + return contains_flag; + } + /* increment depth and potentially safe depth, allocating a new HashMap for this depth's expressions @@ -682,6 +809,177 @@ impl AvailableExpressions { } } +/* + modified for available expression detection purposes. +*/ +trait ExpressionVisitor { + /// Executed before visiting the children of a [ir::Seq] node. + fn start_seq(&mut self, _s: &mut ir::Seq) -> VisResult { + Ok(Action::Continue) + } + + /// Executed after visiting the children of a [ir::Seq] node. + fn finish_seq(&mut self, _s: &mut ir::Seq) -> VisResult { + Ok(Action::Continue) + } + + /// Executed before visiting the children of a [ir::Par] node. + fn start_par(&mut self, _s: &mut ir::Par) -> VisResult { + Ok(Action::Continue) + } + + /// Executed after visiting the children of a [ir::Par] node. + fn finish_par(&mut self, _s: &mut ir::Par) -> VisResult { + Ok(Action::Continue) + } + + /// Executed before visiting the children of a [ir::If] node. + fn start_if(&mut self, _s: &mut ir::If) -> VisResult { + Ok(Action::Continue) + } + + /// Executed after visiting the children of a [ir::If] node. + fn finish_if(&mut self, _s: &mut ir::If) -> VisResult { + Ok(Action::Continue) + } + + /// Executed before visiting the children of a [ir::While] node. + fn start_while(&mut self, _s: &mut ir::While) -> VisResult { + Ok(Action::Continue) + } + + /// Executed after visiting the children of a [ir::While] node. + fn finish_while(&mut self, _s: &mut ir::While) -> VisResult { + Ok(Action::Continue) + } + + /// Executed before visiting the children of a [ir::Repeat] node. + fn start_repeat(&mut self, _s: &mut ir::Repeat) -> VisResult { + Ok(Action::Continue) + } + + /// Executed after visiting the children of a [ir::Repeat] node. + fn finish_repeat(&mut self, _s: &mut ir::Repeat) -> VisResult { + Ok(Action::Continue) + } + + /// Executed at an [ir::Enable] node. + fn enable(&mut self, _s: &mut ir::Enable) -> VisResult { + Ok(Action::Continue) + } + + /// Executed at an [ir::Empty] node. + fn empty(&mut self, _s: &mut ir::Empty) -> VisResult { + Ok(Action::Continue) + } + + /// Executed at an [ir::Invoke] node. + fn invoke(&mut self, _s: &mut ir::Invoke) -> VisResult { + Ok(Action::Continue) + } +} + +// grabbing all of the private +impl Action { + /// Run the traversal specified by `next` if this traversal succeeds. + /// If the result of this traversal is not `Action::Continue`, do not + /// run `next()`. + fn and_then_local(self, mut next: F) -> VisResult + where + F: FnMut() -> VisResult, + { + match self { + Action::Continue => next(), + Action::Change(_) + | Action::Stop + | Action::SkipChildren + | Action::StaticChange(_) => Ok(self), + } + } + /// Applies the Change action if `self` is a Change action. + /// Otherwise passes the action through unchanged + fn apply_change_local(self, con: &mut Control) -> Action { + match self { + Action::Change(c) => { + *con = *c; + Action::Continue + } + action => action, + } + } + /// Changes a Action::SkipChildren to Action::Continue. + /// Should be called to indicate the boundary of traversing the children + /// of a node. + fn pop_local(self) -> Self { + match self { + Action::SkipChildren => Action::Continue, + x => x, + } + } +} + +trait ExpressionVisitable { + /// Perform the traversal. + fn visit(&mut self, visitor: &mut dyn ExpressionVisitor) -> VisResult; +} + +impl ExpressionVisitable for Control { + fn visit(&mut self, visitor: &mut dyn ExpressionVisitor) -> VisResult { + let res = match self { + Control::Seq(ctrl) => visitor + .start_seq(ctrl)? + .and_then_local(|| ctrl.stmts.visit(visitor))? + .pop_local() + .and_then_local(|| visitor.finish_seq(ctrl))?, + Control::Par(ctrl) => visitor + .start_par(ctrl)? + .and_then_local(|| ctrl.stmts.visit(visitor))? + .pop_local() + .and_then_local(|| visitor.finish_par(ctrl))?, + Control::If(ctrl) => visitor + .start_if(ctrl)? + .and_then_local(|| ctrl.tbranch.visit(visitor))? + .and_then_local(|| ctrl.fbranch.visit(visitor))? + .pop_local() + .and_then_local(|| visitor.finish_if(ctrl))?, + Control::While(ctrl) => visitor + .start_while(ctrl)? + .and_then_local(|| ctrl.body.visit(visitor))? + .pop_local() + .and_then_local(|| visitor.finish_while(ctrl))?, + Control::Repeat(ctrl) => visitor + .start_repeat(ctrl)? + .and_then_local(|| ctrl.body.visit(visitor))? + .pop_local() + .and_then_local(|| visitor.finish_repeat(ctrl))?, + Control::Enable(ctrl) => visitor.enable(ctrl)?, + Control::Static(_) => panic!("not supported yet"), + Control::Empty(ctrl) => visitor.empty(ctrl)?, + Control::Invoke(data) => visitor.invoke(data)?, + }; + Ok(res.apply_change_local(self)) + } +} + +/// Blanket implementation for Vectors of Visitables +impl ExpressionVisitable for Vec { + fn visit(&mut self, visitor: &mut dyn ExpressionVisitor) -> VisResult { + for t in self { + let res = t.visit(visitor)?; + match res { + Action::Continue + | Action::SkipChildren + | Action::Change(_) + | Action::StaticChange(_) => { + continue; + } + Action::Stop => return Ok(Action::Stop), + }; + } + Ok(Action::Continue) + } +} + impl Visitor for CseExp { /* Start is treated like a seq block, so simple safe increment @@ -693,32 +991,70 @@ impl Visitor for CseExp { _sigs: &ir::LibrarySignatures, _comps: &[ir::Component], ) -> VisResult { - log::debug!("toplevel start"); + log::debug!("[START] Start AvailableExpression Analysis"); // create depth 0 dictionary, this is basically a seq block self.available_expressions.inc_depth(true); + // Create a clone of the reference to the Control + // program. + let control_ref = Rc::clone(&_comp.control); + if let Control::Empty(_) = &*control_ref.borrow() { + // Don't traverse if the control program is empty. + return Ok(Action::Continue); + } + // Mutably borrow the control program and traverse. + control_ref.borrow_mut().visit(self)?; + // dont need the real visitor to be called here Ok(Action::Continue) } - fn start_seq( + + /* + Remove the identified redundant common subexpressions in each group + */ + fn finish( &mut self, - _s: &mut calyx_ir::Seq, _comp: &mut calyx_ir::Component, _sigs: &calyx_ir::LibrarySignatures, _comps: &[calyx_ir::Component], ) -> VisResult { - log::debug!("start_seq"); - self.available_expressions.inc_depth(true); + log::debug!("[FINISH] Optimize"); + for group in _comp.get_groups_mut().iter_mut() { + let group_name = group.borrow().name().to_string(); + log::debug!("Group: {group_name}"); + self.available_expressions + .optimize(&mut group.borrow_mut(), group_name); + } Ok(Action::Continue) } - fn finish_seq( - &mut self, - _s: &mut calyx_ir::Seq, - _comp: &mut calyx_ir::Component, - _sigs: &calyx_ir::LibrarySignatures, - _comps: &[calyx_ir::Component], - ) -> VisResult { - log::debug!("finish_seq"); - self.available_expressions.dec_depth(); - Ok(Action::Continue) +} + +impl ExpressionVisitor for CseExp { + fn start_if(&mut self, _s: &mut calyx_ir::If) -> VisResult { + log::debug!("start_if"); + // need to run both branches separately and combine common outputs. + let mut true_cse_exp = CseExp { + available_expressions: self.available_expressions.clone(), + }; + let mut false_cse_exp = CseExp { + available_expressions: self.available_expressions.clone(), + }; + log::debug!("running true branch"); + let _ = _s.tbranch.visit(&mut true_cse_exp); + log::debug!("running false branch"); + let _ = _s.fbranch.visit(&mut false_cse_exp); + log::debug!("intersecting branches"); + let (intersection_running, intersection_group) = + AvailableExpressions::intersection( + &true_cse_exp.available_expressions, + &false_cse_exp.available_expressions, + ); + // finally overwrite the current available expressions + log::debug!("overwriting local expressions with branch merge"); + self.available_expressions.running_expressions.insert( + self.available_expressions.current_depth.clone(), + intersection_running, + ); + self.available_expressions.per_group_expressions = intersection_group; + Ok(Action::SkipChildren) } /* Do: @@ -731,13 +1067,7 @@ impl Visitor for CseExp { 3.1) adding the intersection of current running expressions /w this groups logged expressions */ - fn enable( - &mut self, - _s: &mut calyx_ir::Enable, - _comp: &mut calyx_ir::Component, - _sigs: &calyx_ir::LibrarySignatures, - _comps: &[calyx_ir::Component], - ) -> VisResult { + fn enable(&mut self, _s: &mut calyx_ir::Enable) -> VisResult { let group = &_s.group; let group_name = group.borrow().name().to_string(); log::debug!("group {group_name} enable"); @@ -748,23 +1078,4 @@ impl Visitor for CseExp { self.available_expressions.group_exp(group_name.clone()); Ok(Action::Continue) } - - /* - Remove the identified redundant common subexpressions in each group - */ - fn finish( - &mut self, - _comp: &mut calyx_ir::Component, - _sigs: &calyx_ir::LibrarySignatures, - _comps: &[calyx_ir::Component], - ) -> VisResult { - log::debug!("optimize"); - for group in _comp.get_groups_mut().iter_mut() { - let group_name = group.borrow().name().to_string(); - log::debug!("group {group_name}"); - self.available_expressions - .optimize(&mut group.borrow_mut(), group_name); - } - Ok(Action::Continue) - } } From 4ab755fd887f48487dd6b981c3680b10574441a4 Mon Sep 17 00:00:00 2001 From: Ayana Alemayehu Date: Tue, 13 May 2025 20:33:46 -0400 Subject: [PATCH 5/7] massive refactor, added intersection and union methods and enable_addition flag, first (hopefully) correct take at par blocks respecting their all-at-once nature --- calyx/opt/src/passes_experimental/cse_exp.rs | 760 +++++++++---------- 1 file changed, 347 insertions(+), 413 deletions(-) diff --git a/calyx/opt/src/passes_experimental/cse_exp.rs b/calyx/opt/src/passes_experimental/cse_exp.rs index b65fafd924..fba9257645 100644 --- a/calyx/opt/src/passes_experimental/cse_exp.rs +++ b/calyx/opt/src/passes_experimental/cse_exp.rs @@ -1,7 +1,7 @@ use crate::traversal::{Action, Named, VisResult, Visitor}; use calyx_ir::{self as ir}; use ir::Control::{self as Control}; -use std::{collections::HashMap, rc::Rc, string}; +use std::{collections::HashMap, rc::Rc}; const SUPPORTED_STD: &[&str] = &["std_add"]; @@ -15,7 +15,7 @@ impl Named for CseExp { } fn description() -> &'static str { - "replace common subexpression uses with already computed values when possible" + "constant focused common subexpression elimination" } } @@ -23,12 +23,9 @@ impl Default for CseExp { fn default() -> Self { CseExp { available_expressions: AvailableExpressions { - current_depth: -1, - safe_depth: -1, - running_expressions: HashMap::< - i32, - HashMap>, - >::new(), + enable_addition: true, + running_expressions: + HashMap::>::new(), per_group_expressions: HashMap::< String, HashMap>, @@ -53,9 +50,8 @@ impl Clone for ExpressionMetadata { } struct AvailableExpressions { - current_depth: i32, - safe_depth: i32, - running_expressions: HashMap>>, // its a vector to deal with duplicates + enable_addition: bool, // true enables full add_exp, kill_exp, group_exp, false only allows kill_exp + running_expressions: HashMap>, // its a vector to deal with duplicates per_group_expressions: HashMap>>, } @@ -77,33 +73,54 @@ impl AvailableExpressions { } } - pub fn intersection( + // unions two available expression structs + pub fn union( one: &AvailableExpressions, two: &AvailableExpressions, ) -> ( HashMap>, HashMap>>, ) { - // <- i was working on an intersection function to take two available expressions structs/objects and create a new hashmap representing their intersections (to then finish the correct if implementation) - // we follow one's order of stuff for the intersection, since it has to exist in both branches its an alright order to follow - let mut running_intersection = - HashMap::>::new(); - let mut group_intersection = - HashMap::>>::new(); - for (_, expressions) in one.running_expressions.iter() { - for (string_expression, metadata_vec) in expressions { - let mut new_metadata_vec = Vec::::new(); + // we follow one's order of stuff for the union, i dont know of anything better + // two's additions will be after one's if they share the same value + let mut running_union = one.running_expressions.clone(); + for (string_expression, metadata_vec) in two.running_expressions.iter() + { + if !one.running_expressions.contains_key(string_expression) { + running_union + .insert(string_expression.clone(), metadata_vec.clone()); + } else { for metadata in metadata_vec { - if two.contains_metadata(&string_expression, &metadata) { + if !one + .running_contains_metadata(string_expression, metadata) + { + // this is a new metadata in a shared string_expression + let mut new_metadata_vec = running_union + .get(string_expression) + .expect("should contain metadata_vec") + .clone(); new_metadata_vec.push(metadata.clone()); + running_union.insert( + string_expression.clone(), + new_metadata_vec, + ); } } - if new_metadata_vec.len() > 0 { - running_intersection - .insert(string_expression.clone(), new_metadata_vec); - } } } + // this is still a group intersection + let group_intersection = + AvailableExpressions::group_intersection(one, two); + return (running_union, group_intersection); + } + + // intersects two available expression structs' per_group expressions + pub fn group_intersection( + one: &AvailableExpressions, + two: &AvailableExpressions, + ) -> HashMap>> { + let mut group_intersection = + HashMap::>>::new(); for (group, expressions) in one.per_group_expressions.iter() { // do intersection if both available expressions have the group if two.per_group_expressions.contains_key(group) { @@ -154,41 +171,63 @@ impl AvailableExpressions { group_intersection.insert(group.clone(), expressions.clone()); } } + return group_intersection; + } + + // intersects two availalbe expression structs + pub fn intersection( + one: &AvailableExpressions, + two: &AvailableExpressions, + ) -> ( + HashMap>, + HashMap>>, + ) { + // we follow one's order of stuff for the intersection, since it has to exist in both branches its an alright order to follow + let mut running_intersection = + HashMap::>::new(); + for (string_expression, metadata_vec) in one.running_expressions.iter() + { + let mut new_metadata_vec = Vec::::new(); + for metadata in metadata_vec { + if two.running_contains_metadata(&string_expression, &metadata) + { + new_metadata_vec.push(metadata.clone()); + } + } + if new_metadata_vec.len() > 0 { + running_intersection + .insert(string_expression.clone(), new_metadata_vec); + } + } + let group_intersection = + AvailableExpressions::group_intersection(one, two); return (running_intersection, group_intersection); } fn clone(&self) -> AvailableExpressions { AvailableExpressions { - current_depth: self.current_depth, - safe_depth: self.safe_depth, + enable_addition: self.enable_addition, running_expressions: self.running_expressions.clone(), per_group_expressions: self.per_group_expressions.clone(), } } // for checking running_expressions - fn contains_metadata( + fn running_contains_metadata( &self, string_expression: &String, metadata: &ExpressionMetadata, ) -> bool { let mut contains_flag = false; - for depth in 0..(self.current_depth + 1) { - match self.running_expressions.get(&depth) { - Some(expressions) => match expressions.get(string_expression) { - Some(metadata_vec) => { - for met in metadata_vec { - if met.group == metadata.group - && met.reg_port == metadata.reg_port - { - contains_flag = true; - } - } + match self.running_expressions.get(string_expression) { + Some(metadata_vec) => { + for met in metadata_vec { + if met.group == metadata.group + && met.reg_port == metadata.reg_port + { + contains_flag = true; } - None => {} - }, - None => { - panic!("no HashMap allocated for depth {depth}?"); } } + None => {} } return contains_flag; } @@ -224,80 +263,19 @@ impl AvailableExpressions { } /* - increment depth and potentially safe depth, allocating - a new HashMap for this depth's expressions - */ - fn inc_depth(&mut self, safe_depth: bool) -> () { - // invariant check - assert!( - self.safe_depth <= self.current_depth, - "safe depth somehow exceeded current depth" - ); - // only increment if current_depth is on par with safe depth, - // otherwise either safe_depth is false OR we are in an unsafe - // zone. - if safe_depth && self.current_depth == self.safe_depth { - self.current_depth += 1; - self.safe_depth += 1; - } else { - self.current_depth += 1; - } - // this key should never exist already - let dbg_depth = self.current_depth; - if self.running_expressions.contains_key(&self.current_depth) { - let dbg_map_len = - match self.running_expressions.get(&self.current_depth) { - Some(v) => v.keys().len(), - None => panic! {"??"}, - }; - panic!( - "running expressions somehow already contains current depth key {dbg_depth} len {dbg_map_len}" - ); - } - let current_depth_expressions: HashMap< - String, - Vec, - > = HashMap::>::new(); - self.running_expressions - .insert(self.current_depth, current_depth_expressions); - log::debug!( - "incremented current depth to {dbg_depth} and allocated hashmap for expressions" - ); - } - /* - decrement depth and potentially safe depth, deleting the HashMap - allocated for this depth's expressions - */ - fn dec_depth(&mut self) -> () { - let deleted_depth = self.current_depth; - // invariant check - assert!( - self.safe_depth <= self.current_depth, - "safe depth somehow exceeded current depth" - ); - if self.current_depth == self.safe_depth { - self.safe_depth -= 1; - self.current_depth -= 1; - } else { - self.current_depth -= 1; - } - let dbg_depth = self.current_depth; - if self.running_expressions.remove(&deleted_depth).is_some() { - log::debug!( - "decremented current depth to {dbg_depth} and removed hashmap for expressions at depth {deleted_depth}" - ); - } - } - - /* - add to current_depth's running_expressions available subexpressions from - supported operations + given a list of assignments within a group, add to running_expressions + the corresponding available subexpressions in said group + (restricted to supported operations) */ fn add_exp( &mut self, assignments: &Vec>, // a specific group's assignments group: String, // the group with the assignments in question ) -> () { + if !self.enable_addition { + log::debug!("enable addition is false, not running add_exp"); + return; + } let mut intermediate_exp: HashMap = HashMap::::new(); let mut completed_exp = HashMap::::new(); @@ -313,6 +291,7 @@ impl AvailableExpressions { }; if !(SUPPORTED_STD.contains(&operation.to_string().as_str())) { // here we check if a register is latching an existing subexpression + // (since this isn't a supported operation to track common subexpression elimination) let dst_port_name = assign.dst.borrow().name; if operation.to_string().as_str() == "std_reg" && dst_port_name.to_string().as_str() == "in" @@ -325,107 +304,81 @@ impl AvailableExpressions { ); if completed_exp .contains_key(&latching_cadidate.to_string()) - && self.current_depth <= self.safe_depth && src_port_name.to_string().as_str() == "out" { - let string_expression = match completed_exp - .get(&latching_cadidate.to_string()) - { - Some(v) => v, - None => panic!( - "expected completed expressions to contain latching candidaate string" - ), - }; + let string_expression = completed_exp + .get(&latching_cadidate.to_string()).expect("expected completed expressions to contain latching candidate string"); let new_exp = ExpressionMetadata { reg_port: assign .dst .borrow() .cell_parent() .borrow() - .get("out"), // <------ right now only the in port is written down, we need it to be the .out port. TODO + .get("out") + .clone(), group: group.clone(), }; + let dbg_parent = new_exp + .reg_port + .borrow() + .cell_parent() + .borrow() + .name(); + let dbg_port = new_exp.reg_port.borrow().name; match self .running_expressions - .get_mut(&self.current_depth) + .get_mut(string_expression) { - Some(current_depth_expressions) => { - let dbg_depth: i32 = self.current_depth; - let dbg_parent = new_exp - .reg_port - .borrow() - .cell_parent() - .borrow() - .name(); - let dbg_port = new_exp.reg_port.borrow().name; - match current_depth_expressions - .get_mut(string_expression) - { - Some(string_expression_sources) => { - // existing list of subexpressions will have another source appended on it - log::debug!( - "[GEN] adding {string_expression} with parent port {dbg_parent}.{dbg_port} to existing list at depth {dbg_depth}" - ); - string_expression_sources.push(new_exp); - } - None => { - // new list of subexpressions will be allocated - let new_exp_sources = vec![new_exp]; - log::debug!( - "[GEN] adding expression {string_expression} with parent port {dbg_parent}.{dbg_port} to new list of running expressions at depth {dbg_depth}" - ); - current_depth_expressions.insert( - string_expression.to_string(), - new_exp_sources, - ); - } - } + Some(metadata_vec) => { + // existing list of subexpressions will have another source appended on it + log::debug!( + "[GEN] adding {string_expression} with parent port {dbg_parent}.{dbg_port} to existing list" + ); + metadata_vec.push(new_exp); } None => { - panic!( - "current depth not found in running expressions" + // new list of subexpressions will be allocated + let new_metadata_vec = vec![new_exp]; + log::debug!( + "[GEN] adding expression {string_expression} with parent port {dbg_parent}.{dbg_port} to new list of running expressions" + ); + self.running_expressions.insert( + string_expression.to_string(), + new_metadata_vec, ); } } } } - // TODO: ensure expresion is latched and safe_depth is >= cur_depth before adding to avaialble expresions - continue; - } - // check intermediate_exp if already contains expression - let operation_cell_name = - assign.dst.borrow().cell_parent().borrow().name(); - if !intermediate_exp.contains_key(&operation_cell_name.to_string()) - { - intermediate_exp.insert( - operation_cell_name.to_string(), - AvailableExpressions::get_val(&assign.src.borrow()), - ); - continue; - } - // else we have completed this subexpression - else { - let dest = - intermediate_exp.get(&operation_cell_name.to_string()); - match dest { - Some(destination) => { - // grab full subexpression - let cdepth = self.current_depth; - let source = - AvailableExpressions::get_val(&assign.src.borrow()); - let expression = - format!("{source}({operation}){destination}"); - log::debug!( - "added {expression} for depth {cdepth} to completed (intermediate) expressions" - ); - completed_exp.insert( - operation_cell_name.to_string(), - expression, - ); - } - None => { - panic!("missing key?"); - } + } else { + // this is a supported operation to track common subexpression elimination + + // check intermediate_exp if already contains expression + let operation_cell_name = + assign.dst.borrow().cell_parent().borrow().name(); + if !intermediate_exp + .contains_key(&operation_cell_name.to_string()) + { + intermediate_exp.insert( + operation_cell_name.to_string(), + AvailableExpressions::get_val(&assign.src.borrow()), + ); + } + // else we have completed this subexpression + else { + let destination = intermediate_exp + .get(&operation_cell_name.to_string()) + .expect("missing intermediate expression key"); + // grab full subexpression + let source = + AvailableExpressions::get_val(&assign.src.borrow()); + let expression = + format!("{source}({operation}){destination}"); + log::debug!( + "added {expression} to completed (intermediate) expressions" + ); + completed_exp + .insert(operation_cell_name.to_string(), expression); } } } @@ -433,15 +386,15 @@ impl AvailableExpressions { /* identify destroyed expressions from register overwrites - and remove from all depths. + and remove from running_expressions */ fn kill_exp( &mut self, assignments: &Vec>, group: String, ) { - // let mut remove_expressions: Vec = Vec::new(); for assign in assignments.iter() { + // early breakouts if assign.dst.borrow().is_hole() { continue; } @@ -450,78 +403,52 @@ impl AvailableExpressions { Some(v) => v, None => continue, }; + // we need to see if a register that is containing a currently latched // subexpression is being overwritted let dst_port = assign.dst.borrow(); if operation.to_string().as_str() == "std_reg" && dst_port.name.to_string().as_str() == "in" { - for depth in 0..(self.current_depth + 1) { - let e = self.running_expressions.get(&depth); - let mut updates = - HashMap::>::new(); - match e { - Some(expressions) => { - for (string_expression, metadata_vec) in - expressions.into_iter() - { - let mut new_expression_sources = - Vec::::new(); - for metadata in metadata_vec.into_iter() { - // either this was introduced in this group, or we don't share a cell_parent - // if the above is true then we keep the expression - if metadata.group == group - || metadata - .reg_port - .borrow() - .cell_parent() - .borrow() - .name() - != dst_port - .cell_parent() - .borrow() - .name() - { - new_expression_sources.push( - ExpressionMetadata { - reg_port: metadata - .reg_port - .clone(), - group: metadata.group.clone(), - }, - ); - } else { - let dbg_parent = dst_port - .cell_parent() - .borrow() - .name(); - let dbg_port = - metadata.reg_port.borrow().name; - log::debug!( - "[KILL] removed {string_expression} with parent port {dbg_parent}.{dbg_port} from expressions at depth {depth}" - ); - } - } - updates.insert( - string_expression.clone(), - new_expression_sources, - ); - } - } - None => { - panic!("no HashMap allocated for depth {depth}?"); - } - } - match self.running_expressions.get_mut(&depth) { - Some(expressions) => { - for (key, value) in updates { - expressions.insert(key, value); - } - } - None => { - panic!("no HashMap allocated for depth {depth}?"); + let mut updates = + HashMap::>::new(); + for (string_expression, metadata_vec) in + self.running_expressions.iter() + { + let mut new_expression_sources = + Vec::::new(); + for metadata in metadata_vec { + // either this was introduced in this group, or we don't share a cell_parent + // if the above is true then we keep the expression + if metadata.group == group + || metadata + .reg_port + .borrow() + .cell_parent() + .borrow() + .name() + != dst_port.cell_parent().borrow().name() + { + new_expression_sources.push(ExpressionMetadata { + reg_port: metadata.reg_port.clone(), + group: metadata.group.clone(), + }); + } else { + let dbg_parent = + dst_port.cell_parent().borrow().name(); + let dbg_port = metadata.reg_port.borrow().name; + log::debug!( + "[KILL] removed {string_expression} with parent port {dbg_parent}.{dbg_port} from expressions" + ); } } + updates.insert( + string_expression.clone(), + new_expression_sources, + ); + } + for (key, value) in updates { + self.running_expressions.insert(key, value); } } } @@ -534,113 +461,87 @@ impl AvailableExpressions { 2) else, do self.group_expressions[group] ∩ self.running_expressions */ fn group_exp(&mut self, group: String) { - if self.per_group_expressions.contains_key(&group) { - // do 2) - let mut new_group_expressions = - HashMap::>::new(); - // let mut remove_expressions: Vec = Vec::new(); - let g_e = self.per_group_expressions.get(&group); - match g_e { - Some(group_expressions) => { - for (group_string_expression, metadata_vec) in - group_expressions - { - let mut new_group_expression_vec = - Vec::::new(); - for metadata in metadata_vec.into_iter() { - if self.contains_metadata( - group_string_expression, - metadata, - ) { - new_group_expression_vec.push( - ExpressionMetadata { - group: metadata.group.clone(), - reg_port: metadata.reg_port.clone(), - }, - ) - } else { - let dbg_parent = metadata - .reg_port - .borrow() - .cell_parent() - .borrow() - .name(); - let dbg_port = metadata.reg_port.borrow().name; - log::debug!( - "[GROUP-KILL] removed expression {group_string_expression} with parent port {dbg_parent}.{dbg_port}" - ); - } - } - new_group_expressions.insert( - group_string_expression.clone(), - new_group_expression_vec, - ); - } - } - None => { - panic!("expected group expressions to exist"); - } - } - self.per_group_expressions - .insert(group.clone(), new_group_expressions); - } else { + if !self.enable_addition { + log::debug!("enable_addition is false, not running group_exp()"); + return; + } + if !self.per_group_expressions.contains_key(&group) { // do 1) let mut new_group_expressions: HashMap< String, Vec, > = HashMap::>::new(); - for depth in 0..(self.current_depth + 1) { - let e = self.running_expressions.get_mut(&depth); - match e { - Some(expressions) => { - for (string_expression, metadata_vec) in - expressions.into_iter() - { - for metadata in metadata_vec.into_iter() { - match new_group_expressions - .get_mut(string_expression) - { - Some(group_expression_vec) => { - group_expression_vec.push( - ExpressionMetadata { - reg_port: metadata - .reg_port - .clone(), - group: metadata.group.clone(), - }, - ) - } - None => { - let mut new_group_expression_vec = - Vec::::new(); - new_group_expression_vec.push( - ExpressionMetadata { - reg_port: metadata - .reg_port - .clone(), - group: metadata.group.clone(), - }, - ); - new_group_expressions.insert( - string_expression.clone(), - new_group_expression_vec, - ); - } - } - } + for (string_expression, metadata_vec) in + self.running_expressions.iter() + { + for metadata in metadata_vec.into_iter() { + match new_group_expressions.get_mut(string_expression) { + Some(group_expression_vec) => group_expression_vec + .push(ExpressionMetadata { + reg_port: metadata.reg_port.clone(), + group: metadata.group.clone(), + }), + None => { + let mut new_group_expression_vec = + Vec::::new(); + new_group_expression_vec.push(ExpressionMetadata { + reg_port: metadata.reg_port.clone(), + group: metadata.group.clone(), + }); + new_group_expressions.insert( + string_expression.clone(), + new_group_expression_vec, + ); } } - None => { - panic!("no HashMap allocated for depth {depth}?"); - } } } - let dbg_depth = self.current_depth; log::debug!( - "[GROUP-GEN] inserted all running expressions from depth {dbg_depth} downwards for group {group}" + "[GROUP-GEN] inserted all running expressions for group {group}" ); self.per_group_expressions .insert(group, new_group_expressions); + } else { + // do 2) + let mut new_group_expressions = + HashMap::>::new(); + + let group_expressions = self + .per_group_expressions + .get(&group) + .expect(&format!("expected {group} expressions to exist")); + for (group_string_expression, metadata_vec) in group_expressions { + let mut new_group_expression_vec = + Vec::::new(); + for metadata in metadata_vec.into_iter() { + if self.running_contains_metadata( + group_string_expression, + metadata, + ) { + new_group_expression_vec.push(ExpressionMetadata { + group: metadata.group.clone(), + reg_port: metadata.reg_port.clone(), + }) + } else { + let dbg_parent = metadata + .reg_port + .borrow() + .cell_parent() + .borrow() + .name(); + let dbg_port = metadata.reg_port.borrow().name; + log::debug!( + "[GROUP-KILL] removed expression {group_string_expression} with parent port {dbg_parent}.{dbg_port}" + ); + } + } + new_group_expressions.insert( + group_string_expression.clone(), + new_group_expression_vec, + ); + } + self.per_group_expressions + .insert(group.clone(), new_group_expressions); } } /* @@ -655,7 +556,7 @@ impl AvailableExpressions { fn optimize( &mut self, group_obj: &mut std::cell::RefMut, - group: String, // the group with the assignments in question + group: String, ) -> () { let mut intermediate_exp: HashMap = HashMap::::new(); @@ -707,17 +608,9 @@ impl AvailableExpressions { Some(v) => v, None => continue, }; - let current_group_subexpressions = match self + let current_group_subexpressions = self .per_group_expressions - .get(&group) - { - Some(v) => v, - None => { - panic!( - "group should have available expressions at this point" - ) - } - }; + .get(&group).expect(&format!("{group} should have available expressions at this point")); match current_group_subexpressions.get(string_expression) { Some(potential_common_subexpression_vec) => { if potential_common_subexpression_vec.len() > 0 { @@ -761,48 +654,39 @@ impl AvailableExpressions { rewriter.rewrite_assign(&mut asgn); *assign = asgn; } - } else { - continue; } } None => continue, } - continue; - } - // check intermediate_exp if already contains expression - let operation_cell_name = - assign.dst.borrow().cell_parent().borrow().name(); - if !intermediate_exp.contains_key(&operation_cell_name.to_string()) - { - intermediate_exp.insert( - operation_cell_name.to_string(), - AvailableExpressions::get_val(&assign.src.borrow()), - ); - continue; - } - // else we have completed this subexpression - else { - let dest = - intermediate_exp.get(&operation_cell_name.to_string()); - match dest { - Some(destination) => { - // grab full subexpression - let cdepth = self.current_depth; - let source = - AvailableExpressions::get_val(&assign.src.borrow()); - let expression = - format!("{source}({operation}){destination}"); - log::debug!( - "added {expression} for depth {cdepth} to completed (intermediate) expressions" - ); - completed_exp.insert( - operation_cell_name.to_string(), - expression, - ); - } - None => { - panic!("missing key?"); - } + } else { + // this is a supported operation to track common subexpression elimination + + // check intermediate_exp if already contains expression + let operation_cell_name = + assign.dst.borrow().cell_parent().borrow().name(); + if !intermediate_exp + .contains_key(&operation_cell_name.to_string()) + { + intermediate_exp.insert( + operation_cell_name.to_string(), + AvailableExpressions::get_val(&assign.src.borrow()), + ); + continue; + } + // else we have completed this subexpression + else { + let destination = intermediate_exp + .get(&operation_cell_name.to_string()) + .expect("missing intermediate expression key"); + let source = + AvailableExpressions::get_val(&assign.src.borrow()); + let expression = + format!("{source}({operation}){destination}"); + log::debug!( + "added {expression} to completed (intermediate) expressions" + ); + completed_exp + .insert(operation_cell_name.to_string(), expression); } } } @@ -991,9 +875,7 @@ impl Visitor for CseExp { _sigs: &ir::LibrarySignatures, _comps: &[ir::Component], ) -> VisResult { - log::debug!("[START] Start AvailableExpression Analysis"); - // create depth 0 dictionary, this is basically a seq block - self.available_expressions.inc_depth(true); + log::debug!("[START] Toplevel AvailableExpression Analysis"); // Create a clone of the reference to the Control // program. let control_ref = Rc::clone(&_comp.control); @@ -1003,7 +885,7 @@ impl Visitor for CseExp { } // Mutably borrow the control program and traverse. control_ref.borrow_mut().visit(self)?; - // dont need the real visitor to be called here + // can't call skip-children here unfortunately since visitor missing pop() call Ok(Action::Continue) } @@ -1037,9 +919,9 @@ impl ExpressionVisitor for CseExp { let mut false_cse_exp = CseExp { available_expressions: self.available_expressions.clone(), }; - log::debug!("running true branch"); + log::debug!("[START] starting true branch"); let _ = _s.tbranch.visit(&mut true_cse_exp); - log::debug!("running false branch"); + log::debug!("[START] starting false branch"); let _ = _s.fbranch.visit(&mut false_cse_exp); log::debug!("intersecting branches"); let (intersection_running, intersection_group) = @@ -1048,14 +930,66 @@ impl ExpressionVisitor for CseExp { &false_cse_exp.available_expressions, ); // finally overwrite the current available expressions - log::debug!("overwriting local expressions with branch merge"); - self.available_expressions.running_expressions.insert( - self.available_expressions.current_depth.clone(), - intersection_running, - ); + log::debug!("overwriting local expressions with branch intersection"); + self.available_expressions.running_expressions = intersection_running; self.available_expressions.per_group_expressions = intersection_group; Ok(Action::SkipChildren) } + fn start_par(&mut self, _s: &mut calyx_ir::Par) -> VisResult { + log::debug!("start_par"); + // first disable enable_addition and save state + self.available_expressions.enable_addition = false; + let initial_save_state = self.available_expressions.clone(); + // need to run all branches independently and merge their outputs + for control in _s.stmts.iter_mut() { + let mut child_control_cse_exp = CseExp { + available_expressions: initial_save_state.clone(), + }; + log::debug!( + "[START] starting par control child for baseline construction" + ); + let _ = control.visit(&mut child_control_cse_exp); + log::debug!( + "intersection between parent available expression and child control" + ); + let (intersection_running, intersection_group) = + AvailableExpressions::intersection( + &self.available_expressions, + &child_control_cse_exp.available_expressions, + ); + log::debug!( + "overwriting local expressions with child intersection" + ); + self.available_expressions.running_expressions = + intersection_running; + self.available_expressions.per_group_expressions = + intersection_group; + } + // at this point all expressions that would have been killed at any point by a child + // have been killed, and this is reflected in self.available expressions + self.available_expressions.enable_addition = true; + let true_baseline_save_state = self.available_expressions.clone(); + for control in _s.stmts.iter_mut() { + let mut child_control_cse_exp = CseExp { + available_expressions: true_baseline_save_state.clone(), + }; + log::debug!("[START] starting par control child for union"); + let _ = control.visit(&mut child_control_cse_exp); + log::debug!( + "union between parent available expression and child control" + ); + let (union_running, union_group) = AvailableExpressions::union( + &self.available_expressions, + &child_control_cse_exp.available_expressions, + ); + log::debug!( + "overwriting local expressions with child intersection" + ); + self.available_expressions.running_expressions = union_running; + self.available_expressions.per_group_expressions = union_group; + } + Ok(Action::SkipChildren) + } /* Do: 1) add expressions this group creates From b41fd65856c77e9bd4f24704cdb6795ce69818ab Mon Sep 17 00:00:00 2001 From: Ayana Alemayehu Date: Tue, 13 May 2025 22:22:21 -0400 Subject: [PATCH 6/7] found dumb bug in group_intersection, proved my solution works with nested pars (or atleast I hope i proved it, dont see any problems) --- calyx/opt/src/passes_experimental/cse_exp.rs | 124 ++++++++++++++----- 1 file changed, 91 insertions(+), 33 deletions(-) diff --git a/calyx/opt/src/passes_experimental/cse_exp.rs b/calyx/opt/src/passes_experimental/cse_exp.rs index fba9257645..0e6581a18f 100644 --- a/calyx/opt/src/passes_experimental/cse_exp.rs +++ b/calyx/opt/src/passes_experimental/cse_exp.rs @@ -23,7 +23,8 @@ impl Default for CseExp { fn default() -> Self { CseExp { available_expressions: AvailableExpressions { - enable_addition: true, + enable: -1, + current_depth: 0, running_expressions: HashMap::>::new(), per_group_expressions: HashMap::< @@ -50,7 +51,8 @@ impl Clone for ExpressionMetadata { } struct AvailableExpressions { - enable_addition: bool, // true enables full add_exp, kill_exp, group_exp, false only allows kill_exp + enable: i32, // -1 enables full add_exp, kill_exp, group_exp, >= 0 only allows kill_exp + current_depth: i32, // current depth with regards to nested par blocks, loops, anything that toggles enable_addition running_expressions: HashMap>, // its a vector to deal with duplicates per_group_expressions: HashMap>>, @@ -148,14 +150,18 @@ impl AvailableExpressions { { let mut group_metadata_vec = group_expressions.get(string_expression).expect(&format!("expected metadata vec {string_expression}")).clone(); group_metadata_vec.push(metadata.clone()); - group_expressions - .insert(group.clone(), group_metadata_vec); + group_expressions.insert( + string_expression.clone(), + group_metadata_vec, + ); } else { let mut group_metadata_vec = Vec::::new(); group_metadata_vec.push(metadata.clone()); - group_expressions - .insert(group.clone(), group_metadata_vec); + group_expressions.insert( + string_expression.clone(), + group_metadata_vec, + ); } } } @@ -203,9 +209,52 @@ impl AvailableExpressions { AvailableExpressions::group_intersection(one, two); return (running_intersection, group_intersection); } + + fn inc_depth(&mut self) { + self.current_depth += 1; + let dbg_depth = self.current_depth; + log::debug!("incremented depth to {dbg_depth}"); + } + + fn dec_depth(&mut self) { + self.current_depth -= 1; + let dbg_depth = self.current_depth; + log::debug!("decremented depth to {dbg_depth}"); + } + + fn disable_addition(&mut self) { + let dbg_depth = self.current_depth; + if self.enable == -1 { + self.enable = self.current_depth; + log::debug!("disabled addition at depth {dbg_depth}"); + } else { + // otherwise its already disabled + log::debug!("already disabled at depth {dbg_depth}") + } + } + + fn enable_addition(&mut self) -> bool { + let dbg_depth = self.current_depth; + // priority is given to higher scopes (lower number depths) + if self.enable == -1 { + log::debug!("already enabled at depth {dbg_depth}"); + return true; + } else if self.current_depth <= self.enable { + self.enable = -1; + log::debug!("enabled addition at depth {dbg_depth}"); + return true; + } else { + log::debug!( + "couldn't enable addition, higher-level scope holding enable" + ); + return false; + } + } + fn clone(&self) -> AvailableExpressions { AvailableExpressions { - enable_addition: self.enable_addition, + enable: self.enable, + current_depth: self.current_depth, running_expressions: self.running_expressions.clone(), per_group_expressions: self.per_group_expressions.clone(), } @@ -272,8 +321,8 @@ impl AvailableExpressions { assignments: &Vec>, // a specific group's assignments group: String, // the group with the assignments in question ) -> () { - if !self.enable_addition { - log::debug!("enable addition is false, not running add_exp"); + if self.enable != -1 { + log::debug!("disabled addition, not running add_exp"); return; } let mut intermediate_exp: HashMap = @@ -461,8 +510,8 @@ impl AvailableExpressions { 2) else, do self.group_expressions[group] ∩ self.running_expressions */ fn group_exp(&mut self, group: String) { - if !self.enable_addition { - log::debug!("enable_addition is false, not running group_exp()"); + if self.enable != -1 { + log::debug!("disabled addition, not running group_exp()"); return; } if !self.per_group_expressions.contains_key(&group) { @@ -936,9 +985,13 @@ impl ExpressionVisitor for CseExp { Ok(Action::SkipChildren) } fn start_par(&mut self, _s: &mut calyx_ir::Par) -> VisResult { + log::debug!( + "-----------------par baseline construction-----------------" + ); log::debug!("start_par"); - // first disable enable_addition and save state - self.available_expressions.enable_addition = false; + // first inc depth and disable enable_addition and save state + self.available_expressions.inc_depth(); + self.available_expressions.disable_addition(); let initial_save_state = self.available_expressions.clone(); // need to run all branches independently and merge their outputs for control in _s.stmts.iter_mut() { @@ -967,27 +1020,32 @@ impl ExpressionVisitor for CseExp { } // at this point all expressions that would have been killed at any point by a child // have been killed, and this is reflected in self.available expressions - self.available_expressions.enable_addition = true; - let true_baseline_save_state = self.available_expressions.clone(); - for control in _s.stmts.iter_mut() { - let mut child_control_cse_exp = CseExp { - available_expressions: true_baseline_save_state.clone(), - }; - log::debug!("[START] starting par control child for union"); - let _ = control.visit(&mut child_control_cse_exp); - log::debug!( - "union between parent available expression and child control" - ); - let (union_running, union_group) = AvailableExpressions::union( - &self.available_expressions, - &child_control_cse_exp.available_expressions, - ); - log::debug!( - "overwriting local expressions with child intersection" - ); - self.available_expressions.running_expressions = union_running; - self.available_expressions.per_group_expressions = union_group; + log::debug!("-----------------par union-----------------"); + if self.available_expressions.enable_addition() { + let true_baseline_save_state = self.available_expressions.clone(); + for control in _s.stmts.iter_mut() { + let mut child_control_cse_exp = CseExp { + available_expressions: true_baseline_save_state.clone(), + }; + log::debug!("[START] starting par control child for union"); + let _ = control.visit(&mut child_control_cse_exp); + log::debug!( + "union between parent available expression and child control" + ); + let (union_running, union_group) = AvailableExpressions::union( + &self.available_expressions, + &child_control_cse_exp.available_expressions, + ); + log::debug!("overwriting local expressions with child union"); + self.available_expressions.running_expressions = union_running; + self.available_expressions.per_group_expressions = union_group; + } + } else { + log::debug!("exiting early, higher scope will rerun"); } + // dec depth once par block is done + self.available_expressions.dec_depth(); + log::debug!("-----------------par union finished-----------------"); Ok(Action::SkipChildren) } /* From 3b558ce8719d7583788e395a8e4bf5ea7e063888 Mon Sep 17 00:00:00 2001 From: Ayana Alemayehu Date: Thu, 15 May 2025 14:15:53 -0400 Subject: [PATCH 7/7] i think i added loop support successfully, treated loops as a if condition where the loop could run (true branch) or could not (false branch) --- calyx/opt/src/passes_experimental/cse_exp.rs | 100 +++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/calyx/opt/src/passes_experimental/cse_exp.rs b/calyx/opt/src/passes_experimental/cse_exp.rs index 0e6581a18f..f61e4d1755 100644 --- a/calyx/opt/src/passes_experimental/cse_exp.rs +++ b/calyx/opt/src/passes_experimental/cse_exp.rs @@ -1048,6 +1048,106 @@ impl ExpressionVisitor for CseExp { log::debug!("-----------------par union finished-----------------"); Ok(Action::SkipChildren) } + fn start_while(&mut self, _s: &mut calyx_ir::While) -> VisResult { + // similar to par blocks, we create a baseline then run the children with the baseline + // baseline deals with loop running multiple times to ensure available expressions are + // available the entire time + log::debug!("start_while"); + self.available_expressions.inc_depth(); + self.available_expressions.disable_addition(); + let initial_save_state = self.available_expressions.clone(); + let mut body_cse_exp = CseExp { + available_expressions: initial_save_state.clone(), + }; + log::debug!("[START] starting while body for baseline construction"); + let _ = _s.body.visit(&mut body_cse_exp); + log::debug!( + "intersection between parent available expression and body" + ); + let (intersection_running, intersection_group) = + AvailableExpressions::intersection( + &self.available_expressions, + &body_cse_exp.available_expressions, + ); + log::debug!("overwriting local expressions with body intersection"); + self.available_expressions.running_expressions = intersection_running; + self.available_expressions.per_group_expressions = intersection_group; + // now we have a true baseline + if self.available_expressions.enable_addition() { + let true_baseline_save_state = self.available_expressions.clone(); + let mut body_cse_exp = CseExp { + available_expressions: true_baseline_save_state.clone(), + }; + log::debug!("[START] starting while body"); + let _ = _s.body.visit(&mut body_cse_exp); + log::debug!( + "intersection between parent available expression and body" + ); + let (intersection_running, intersection_group) = + AvailableExpressions::intersection( + &self.available_expressions, + &body_cse_exp.available_expressions, + ); + log::debug!("overwriting local expressions with body intersection"); + self.available_expressions.running_expressions = + intersection_running; + self.available_expressions.per_group_expressions = + intersection_group; + } else { + log::debug!("exiting early, higher scope will rerun"); + } + self.available_expressions.dec_depth(); + Ok(Action::SkipChildren) + } + fn start_repeat(&mut self, _s: &mut calyx_ir::Repeat) -> VisResult { + // same as start_while + log::debug!("start_repeat"); + self.available_expressions.inc_depth(); + self.available_expressions.disable_addition(); + let initial_save_state = self.available_expressions.clone(); + let mut body_cse_exp = CseExp { + available_expressions: initial_save_state.clone(), + }; + log::debug!("[START] starting repeat body for baseline construction"); + let _ = _s.body.visit(&mut body_cse_exp); + log::debug!( + "intersection between parent available expression and body" + ); + let (intersection_running, intersection_group) = + AvailableExpressions::intersection( + &self.available_expressions, + &body_cse_exp.available_expressions, + ); + log::debug!("overwriting local expressions with body intersection"); + self.available_expressions.running_expressions = intersection_running; + self.available_expressions.per_group_expressions = intersection_group; + // now we have a true baseline + if self.available_expressions.enable_addition() { + let true_baseline_save_state = self.available_expressions.clone(); + let mut body_cse_exp = CseExp { + available_expressions: true_baseline_save_state.clone(), + }; + log::debug!("[START] starting repeat body"); + let _ = _s.body.visit(&mut body_cse_exp); + log::debug!( + "intersection between parent available expression and body" + ); + let (intersection_running, intersection_group) = + AvailableExpressions::intersection( + &self.available_expressions, + &body_cse_exp.available_expressions, + ); + log::debug!("overwriting local expressions with body intersection"); + self.available_expressions.running_expressions = + intersection_running; + self.available_expressions.per_group_expressions = + intersection_group; + } else { + log::debug!("exiting early, higher scope will rerun"); + } + self.available_expressions.dec_depth(); + Ok(Action::SkipChildren) + } /* Do: 1) add expressions this group creates