From 18419052b89fc888e945b5989b516989554776a9 Mon Sep 17 00:00:00 2001 From: Rohan Kumar Date: Fri, 22 May 2026 15:37:19 -0700 Subject: [PATCH] improve StepRef API and allow multiple step execution --- cadence/src/genus.rs | 9 ++- cadence/src/innovus.rs | 9 ++- cadence/src/pegasus.rs | 9 ++- flows/sky130_cadence/src/lib.rs | 20 +++--- rivet/src/bash.rs | 9 ++- rivet/src/lib.rs | 117 +++++++++++++++++++------------- rivet/src/rust.rs | 11 ++- 7 files changed, 100 insertions(+), 84 deletions(-) diff --git a/cadence/src/genus.rs b/cadence/src/genus.rs index 99b57f4..daff2f1 100644 --- a/cadence/src/genus.rs +++ b/cadence/src/genus.rs @@ -9,8 +9,7 @@ use std::{fs, io}; use crate::{Checkpoint, MmmcConfig, MmmcCorner, SubmoduleInfo, Substep, mmmc}; use fs::File; use indoc::formatdoc; -use rivet::Step; -use std::sync::Arc; +use rivet::{Step, StepRef}; /// Defines the Genus synthesis step subflow #[derive(Debug, Clone)] @@ -21,7 +20,7 @@ pub struct GenusStep { pub pinned: bool, pub start_checkpoint: Option, pub endpoint: Option, - pub dependencies: Vec>, + pub dependencies: Vec>, } impl GenusStep { @@ -30,7 +29,7 @@ impl GenusStep { module: impl Into, steps: Vec, pinned: bool, - deps: Vec>, + deps: Vec>, ) -> Self { let dir = work_dir.into(); let modul = module.into(); @@ -182,7 +181,7 @@ impl Step for GenusStep { } } - fn deps(&self) -> Vec> { + fn deps(&self) -> Vec> { self.dependencies.clone() } diff --git a/cadence/src/innovus.rs b/cadence/src/innovus.rs index 70f250d..4bcc473 100644 --- a/cadence/src/innovus.rs +++ b/cadence/src/innovus.rs @@ -9,11 +9,10 @@ use crate::MmmcCorner; use crate::{Checkpoint, MmmcConfig, SubmoduleInfo, Substep, mmmc}; use fs::File; use indoc::formatdoc; -use rivet::Step; +use rivet::{Step, StepRef}; use rust_decimal::Decimal; use serde::Deserialize; use serde::Serialize; -use std::sync::Arc; /// Defines the Innovus place and route step subflow #[derive(Debug, Clone)] @@ -24,7 +23,7 @@ pub struct InnovusStep { pub pinned: bool, pub start_checkpoint: Option, pub endpoint: Option, - pub deps: Vec>, + pub deps: Vec>, pub synthesis: bool, } @@ -34,7 +33,7 @@ impl InnovusStep { module: impl Into, substeps: Vec, pinned: bool, - deps: Vec>, + deps: Vec>, synthesis: bool, ) -> Self { let dir = work_dir.into(); @@ -204,7 +203,7 @@ impl Step for InnovusStep { } } - fn deps(&self) -> Vec> { + fn deps(&self) -> Vec> { self.deps.clone() } diff --git a/cadence/src/pegasus.rs b/cadence/src/pegasus.rs index 4912ec0..ef4b8d6 100644 --- a/cadence/src/pegasus.rs +++ b/cadence/src/pegasus.rs @@ -6,8 +6,7 @@ use std::{fs, io}; use crate::Substep; use fs::File; -use rivet::Step; -use std::sync::Arc; +use rivet::{Step, StepRef}; #[derive(Debug)] pub struct PegasusStep { @@ -15,7 +14,7 @@ pub struct PegasusStep { pub func: String, pub module: String, pub pinned: bool, - pub dependencies: Vec>, + pub dependencies: Vec>, } impl PegasusStep { @@ -24,7 +23,7 @@ impl PegasusStep { func: String, module: String, pinned: bool, - deps: Vec>, + deps: Vec>, ) -> Self { let dir = work_dir.into(); PegasusStep { @@ -149,7 +148,7 @@ impl Step for PegasusStep { } } } - fn deps(&self) -> Vec> { + fn deps(&self) -> Vec> { self.dependencies.clone() } diff --git a/flows/sky130_cadence/src/lib.rs b/flows/sky130_cadence/src/lib.rs index 8885d3c..785ac45 100644 --- a/flows/sky130_cadence/src/lib.rs +++ b/flows/sky130_cadence/src/lib.rs @@ -343,9 +343,9 @@ pub fn sky130_scl_cadence_syn(config: SclSynConfig<'_>) -> GenusStep { }) .collect(); - let mut deps: Vec> = dep_info + let mut deps: Vec> = dep_info .iter() - .map(|(_module, flow)| Arc::new(flow.par.clone()) as Arc) + .map(|(_module, flow)| flow.par.clone().into_dyn()) .collect(); let is_hierarchical = !submodules.is_empty(); @@ -360,13 +360,13 @@ pub fn sky130_scl_cadence_syn(config: SclSynConfig<'_>) -> GenusStep { fs::create_dir_all(sram_work_dir).expect("Failed to create sram directory"); generate_compiler_script(&missing_srams, sram_work_dir) .expect("Failed to generate SRAM compiler script"); - let sram_compiler = Arc::new(BashStep::new( + let sram_compiler = StepRef::new(BashStep::new( sram_work_dir.to_path_buf(), "generate_sram", module.as_str(), vec![], )); - deps.push(sram_compiler); + deps.push(sram_compiler.into_dyn()); } GenusStep::new( @@ -578,7 +578,7 @@ pub fn sky130_scl_cadence_par(config: SclParConfig<'_>) -> InnovusStep { ), ], matches!(pin_info, FlatPinInfo::PinPar(_)), - vec![Arc::new(syn_step) as Arc], + vec![syn_step.into_dyn()], false, ) } @@ -939,9 +939,9 @@ pub fn sky130_os_cadence_syn(config: OsSynConfig<'_>) -> GenusStep { }) .collect(); - let mut deps: Vec> = dep_info + let mut deps: Vec> = dep_info .iter() - .map(|(_module, flow)| Arc::new(flow.par.clone()) as Arc) + .map(|(_module, flow)| flow.par.clone().into_dyn()) .collect(); let is_hierarchical = !submodules.is_empty(); @@ -956,13 +956,13 @@ pub fn sky130_os_cadence_syn(config: OsSynConfig<'_>) -> GenusStep { fs::create_dir_all(sram_work_dir).expect("Failed to create sram directory"); generate_compiler_script(&missing_srams, sram_work_dir) .expect("Failed to generate SRAM compiler script"); - let sram_compiler = Arc::new(BashStep::new( + let sram_compiler = StepRef::new(BashStep::new( sram_work_dir.to_path_buf(), "generate_sram", module.as_str(), vec![], )); - deps.push(sram_compiler); + deps.push(sram_compiler.into_dyn()); } GenusStep::new( @@ -1152,7 +1152,7 @@ pub fn sky130_os_cadence_par(config: OsParConfig<'_>) -> InnovusStep { ), ], matches!(pin_info, FlatPinInfo::PinPar(_)), - vec![Arc::new(syn_step) as Arc], + vec![syn_step.into_dyn()], false, ) } diff --git a/rivet/src/bash.rs b/rivet/src/bash.rs index 1f537bd..9cc615b 100644 --- a/rivet/src/bash.rs +++ b/rivet/src/bash.rs @@ -1,10 +1,9 @@ -use crate::Step; +use crate::{Step, StepRef}; use std::fmt::Debug; use std::fs::File; use std::io::{BufRead, BufReader, Write}; use std::path::PathBuf; use std::process::{Command, Stdio}; -use std::sync::Arc; use std::thread; #[derive(Debug, Clone)] @@ -12,7 +11,7 @@ pub struct BashStep { pub work_dir: PathBuf, pub name: String, pub block: String, - pub deps: Vec>, + pub deps: Vec>, pub pinned: bool, } @@ -21,7 +20,7 @@ impl BashStep { work_dir: impl Into, name: impl Into, block: impl Into, - deps: Vec>, + deps: Vec>, ) -> Self { let dir = work_dir.into(); let file = name.into(); @@ -95,7 +94,7 @@ impl Step for BashStep { } } - fn deps(&self) -> Vec> { + fn deps(&self) -> Vec> { self.deps.clone() } diff --git a/rivet/src/lib.rs b/rivet/src/lib.rs index 020fb19..333bec9 100644 --- a/rivet/src/lib.rs +++ b/rivet/src/lib.rs @@ -1,7 +1,9 @@ use by_address::ByAddress; -use std::collections::{HashMap, HashSet}; +use std::any::Any; +use std::collections::HashSet; use std::fmt::Debug; -use std::sync::{Arc, Mutex, MutexGuard}; +use std::ops::Deref; +use std::sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard}; pub mod bash; pub mod rust; @@ -60,45 +62,47 @@ impl Dag { } } -pub trait Step: Debug + Send + Sync { - fn deps(&self) -> Vec>; +pub trait Step: Debug + Any + Send + Sync { + fn deps(&self) -> Vec>; fn pinned(&self) -> bool; fn execute(&self); } -pub fn execute(target: impl Step + 'static) { - let target = Arc::new(target) as Arc; - - let mut executed = HashMap::>, Arc>::new(); - execute_inner(target, &mut executed); +#[derive(Default)] +pub struct Executor { + executed: HashSet>>, } -fn execute_inner( - step: Arc, - executed: &mut HashMap>, Arc>, -) { - println!("Checking status of step {step:?}"); - let step_addr = ByAddress(step.clone()); - if executed.contains_key(&step_addr) { - println!("Step has already been executed, skipping"); - return; +impl Executor { + pub fn new() -> Self { + Self::default() } - if step.pinned() { - println!("Step is pinned, skipping"); - executed.insert(step_addr, Arc::clone(&step)); - return; + pub fn execute(mut self, step: StepRef) -> Self { + self.execute_step(step.into_dyn()); + self } - println!("Executing step dependencies: {:?}", step.deps()); - for dependency in step.deps() { - execute_inner(dependency, executed); + fn execute_step(&mut self, step: StepRef) { + let step_addr = ByAddress(step.clone()); + if self.executed.contains(&step_addr) { + return; + } + if step.read().pinned() { + self.executed.insert(step_addr); + return; + } + let deps = step.read().deps(); + for dep in deps { + self.execute_step(dep); + } + step.read().execute(); + self.executed.insert(ByAddress(step)); } +} - println!("Executing step {step:?}"); - step.execute(); - - executed.insert(ByAddress(step.clone()), Arc::clone(&step)); +pub fn execute(step: StepRef) { + Executor::new().execute(step); } pub fn hierarchical(dag: &Dag, flat_flow_gen: &impl Fn(&M, Vec<(&M, &F)>) -> F) -> Dag { @@ -123,42 +127,59 @@ pub fn hierarchical(dag: &Dag, flat_flow_gen: &impl Fn(&M, Vec<(&M, &F) } } -#[derive(Debug, Clone)] -pub struct StepRef { - inner: Arc>, +#[derive(Debug)] +pub struct StepRef { + inner: Arc>, +} + +impl Clone for StepRef { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + } + } } -impl StepRef { +impl Deref for StepRef { + type Target = RwLock; + + fn deref(&self) -> &Self::Target { + &*self.inner + } +} + +impl StepRef { pub fn new(data: T) -> Self { Self { - inner: Arc::new(Mutex::new(data)), + inner: Arc::new(RwLock::new(data)), } } +} - pub fn lock(&self) -> MutexGuard<'_, T> { - self.inner.lock().unwrap() +impl StepRef { + pub fn read(&self) -> RwLockReadGuard<'_, T> { + self.inner.read().unwrap() + } + + pub fn write(&self) -> RwLockWriteGuard<'_, T> { + self.inner.write().unwrap() } pub fn get(&self, get_fn: impl FnOnce(&T) -> R) -> R { - let inner = self.inner.lock().unwrap(); + let inner = self.inner.read().unwrap(); get_fn(&inner) } pub fn update(&self, update_fn: impl FnOnce(&mut T) -> R) -> R { - let mut inner = self.inner.lock().unwrap(); + let mut inner = self.inner.write().unwrap(); update_fn(&mut inner) } } -impl Step for StepRef { - fn execute(&self) { - self.lock().execute(); - } - fn deps(&self) -> Vec> { - self.lock().deps() - } - - fn pinned(&self) -> bool { - self.lock().pinned() +impl StepRef { + pub fn into_dyn(self) -> StepRef { + StepRef { + inner: self.inner as Arc>, + } } } diff --git a/rivet/src/rust.rs b/rivet/src/rust.rs index fc8df23..fba2ea0 100644 --- a/rivet/src/rust.rs +++ b/rivet/src/rust.rs @@ -1,11 +1,10 @@ -use crate::Step; +use crate::{Step, StepRef}; use std::fmt::Debug; -use std::sync::Arc; #[derive(Clone)] pub struct RustStep { pub rust_fn: F, - pub dependencies: Vec>, + pub dependencies: Vec>, pub pinned: bool, } @@ -20,7 +19,7 @@ impl Debug for RustStep { } impl RustStep { - pub fn new(rust_fn: F, deps: Vec>) -> Self { + pub fn new(rust_fn: F, deps: Vec>) -> Self { RustStep { rust_fn, dependencies: deps, @@ -33,12 +32,12 @@ impl RustStep { } } -impl Step for RustStep { +impl Step for RustStep { fn execute(&self) { (self.rust_fn)(); } - fn deps(&self) -> Vec> { + fn deps(&self) -> Vec> { self.dependencies.clone() }