From 90aed1615e63bc0afa9dbdc2e819bdabf4bda817 Mon Sep 17 00:00:00 2001 From: Jacob Lifshay Date: Wed, 24 Sep 2025 22:28:25 -0700 Subject: [PATCH] WIP converting from cli.rs to build/*.rs --- Cargo.lock | 11 -- Cargo.toml | 1 - crates/fayalite/Cargo.toml | 1 - crates/fayalite/src/build.rs | 211 +++++++++++++++++++++++++++- crates/fayalite/src/build/firrtl.rs | 82 +++++++++++ crates/fayalite/src/cli.rs | 2 +- crates/fayalite/src/firrtl.rs | 32 ++++- 7 files changed, 313 insertions(+), 27 deletions(-) create mode 100644 crates/fayalite/src/build/firrtl.rs diff --git a/Cargo.lock b/Cargo.lock index 221e10c..0ff4521 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -309,7 +309,6 @@ dependencies = [ "jobslot", "num-bigint", "num-traits", - "os_pipe", "petgraph", "serde", "serde_json", @@ -514,16 +513,6 @@ version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" -[[package]] -name = "os_pipe" -version = "1.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ffd2b0a5634335b135d5728d84c5e0fd726954b87111f7506a61c502280d982" -dependencies = [ - "libc", - "windows-sys 0.59.0", -] - [[package]] name = "petgraph" version = "0.8.1" diff --git a/Cargo.toml b/Cargo.toml index 88bdbbe..46d749d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,6 @@ indexmap = { version = "2.5.0", features = ["serde"] } jobslot = "0.2.19" num-bigint = "0.4.6" num-traits = "0.2.16" -os_pipe = "1.2.1" petgraph = "0.8.1" prettyplease = "0.2.20" proc-macro2 = "1.0.83" diff --git a/crates/fayalite/Cargo.toml b/crates/fayalite/Cargo.toml index 3fd6c6d..b3005fd 100644 --- a/crates/fayalite/Cargo.toml +++ b/crates/fayalite/Cargo.toml @@ -25,7 +25,6 @@ hashbrown.workspace = true jobslot.workspace = true num-bigint.workspace = true num-traits.workspace = true -os_pipe.workspace = true petgraph.workspace = true serde_json.workspace = true serde.workspace = true diff --git a/crates/fayalite/src/build.rs b/crates/fayalite/src/build.rs index d34d6e6..9005f78 100644 --- a/crates/fayalite/src/build.rs +++ b/crates/fayalite/src/build.rs @@ -7,7 +7,6 @@ use crate::{ module::Module, util::{HashMap, HashSet, job_server::AcquiredJob}, }; -use clap::{FromArgMatches, Subcommand}; use hashbrown::hash_map::Entry; use petgraph::{ algo::{DfsSpace, kosaraju_scc, toposort}, @@ -29,8 +28,10 @@ use std::{ sync::{Arc, OnceLock, RwLock, RwLockWriteGuard, mpsc}, thread::{self, ScopedJoinHandle}, }; +use tempfile::TempDir; pub mod external; +pub mod firrtl; macro_rules! write_str { ($s:expr, $($rest:tt)*) => { @@ -92,8 +93,6 @@ impl Ord for JobItemName { } } -pub struct CommandLine {} - pub trait JobKind: 'static + Send + Sync + Hash + Eq + fmt::Debug { type Job: 'static + Send + Sync + Hash + Eq + fmt::Debug; fn inputs_and_direct_dependencies<'a>( @@ -779,6 +778,7 @@ impl JobKind for DynJobKind { enum JobGraphNode { Job(DynJob), Item { + #[allow(dead_code, reason = "name used for debugging")] name: JobItemName, source_job: Option, }, @@ -1551,7 +1551,7 @@ impl JobKind for InternalJobKind { } fn from_arg_matches(&self, matches: &mut clap::ArgMatches) -> clap::error::Result { - InternalJob::::from_arg_matches_mut(matches) + clap::FromArgMatches::from_arg_matches_mut(matches) } fn to_command_line(&self, job: &Self::Job) -> Interned<[Interned]> { @@ -1568,11 +1568,11 @@ impl JobKind for InternalJobKind { command_line: Interned<[Interned]>, ) -> clap::error::Result { let cmd = clap::Command::new(Interned::into_inner(program_name_for_internal_jobs())); - let mut matches = InternalJob::::augment_subcommands(cmd) + let mut matches = as clap::Subcommand>::augment_subcommands(cmd) .subcommand_required(true) .arg_required_else_help(true) .try_get_matches_from(command_line.iter().map(|arg| &**arg))?; - InternalJob::::from_arg_matches_mut(&mut matches) + as clap::FromArgMatches>::from_arg_matches_mut(&mut matches) } fn run( @@ -1657,3 +1657,202 @@ impl clap::Subcommand for InternalJob { *name == *Job::subcommand_name() } } + +#[derive(clap::Args)] +#[clap(id = "OutputDir")] +struct OutputDirArgs { + /// the directory to put the generated main output file and associated files in + #[arg(short, long, value_hint = clap::ValueHint::DirPath, required = true)] + output: Option, + #[arg(long, env = "FAYALITE_KEEP_TEMP_DIR")] + keep_temp_dir: bool, +} + +#[derive(Debug, Clone)] +pub struct OutputDir { + output: String, + temp_dir: Option>, + keep_temp_dir: bool, +} + +impl Eq for OutputDir {} + +impl AsRef for OutputDir { + fn as_ref(&self) -> &str { + self.path() + } +} + +impl AsRef for OutputDir { + fn as_ref(&self) -> &std::path::Path { + self.path().as_ref() + } +} + +impl OutputDir { + pub fn path(&self) -> &str { + &self.output + } + pub fn new(output: String) -> Self { + Self { + output, + temp_dir: None, + keep_temp_dir: false, + } + } + pub fn with_keep_temp_dir(output: String, keep_temp_dir: bool) -> Self { + Self { + output, + temp_dir: None, + keep_temp_dir, + } + } + pub fn temp(keep_temp_dir: bool) -> std::io::Result { + let temp_dir = TempDir::new()?; + let output = String::from(temp_dir.path().as_os_str().to_str().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidFilename, + format!( + "temporary directory path is not valid UTF-8: {:?}", + temp_dir.path() + ), + ) + })?); + let temp_dir = if keep_temp_dir { + println!( + "created temporary directory: {}", + temp_dir.into_path().display() + ); + None + } else { + Some(Arc::new(temp_dir)) + }; + Ok(Self { + output, + temp_dir, + keep_temp_dir, + }) + } + pub fn to_args(&self) -> Vec> { + let Self { + output, + temp_dir: _, + keep_temp_dir, + } = self; + let mut retval = Vec::new(); + retval.push(str::intern_owned(format!("--output={output}"))); + if *keep_temp_dir { + retval.push("--keep-temp-dir".intern()); + } + retval + } + fn compare_key(&self) -> (&str, bool, bool) { + let Self { + output, + temp_dir, + keep_temp_dir, + } = self; + (output, temp_dir.is_some(), *keep_temp_dir) + } +} + +impl PartialEq for OutputDir { + fn eq(&self, other: &Self) -> bool { + self.compare_key() == other.compare_key() + } +} + +impl Hash for OutputDir { + fn hash(&self, state: &mut H) { + self.compare_key().hash(state); + } +} + +impl TryFrom for OutputDir { + type Error = clap::Error; + + fn try_from(value: OutputDirArgs) -> Result { + let OutputDirArgs { + output, + keep_temp_dir, + } = value; + match output { + Some(output) => Ok(Self::with_keep_temp_dir(output, keep_temp_dir)), + None => Ok(Self::temp(keep_temp_dir)?), + } + } +} + +impl clap::FromArgMatches for OutputDir { + fn from_arg_matches(matches: &clap::ArgMatches) -> clap::error::Result { + OutputDirArgs::from_arg_matches(matches)?.try_into() + } + + fn from_arg_matches_mut(matches: &mut clap::ArgMatches) -> clap::error::Result { + OutputDirArgs::from_arg_matches_mut(matches)?.try_into() + } + + fn update_from_arg_matches(&mut self, matches: &clap::ArgMatches) -> clap::error::Result<()> { + *self = Self::from_arg_matches(matches)?; + Ok(()) + } + + fn update_from_arg_matches_mut( + &mut self, + matches: &mut clap::ArgMatches, + ) -> clap::error::Result<()> { + *self = Self::from_arg_matches_mut(matches)?; + Ok(()) + } +} + +impl clap::Args for OutputDir { + fn group_id() -> Option { + OutputDirArgs::group_id() + } + + fn augment_args(cmd: clap::Command) -> clap::Command { + OutputDirArgs::augment_args(cmd) + } + + fn augment_args_for_update(cmd: clap::Command) -> clap::Command { + OutputDirArgs::augment_args_for_update(cmd) + } +} + +#[derive(clap::Parser, Debug, Clone, Hash, PartialEq, Eq)] +#[non_exhaustive] +pub struct BaseArgs { + /// the directory to put the generated main output file and associated files in + #[command(flatten)] + pub output: OutputDir, + /// the stem of the generated main output file, e.g. to get foo.v, pass --file-stem=foo + #[arg(long)] + pub file_stem: Option, + pub module_name: String, +} + +impl BaseArgs { + pub fn to_args(&self) -> Vec> { + let Self { + output, + file_stem, + module_name, + } = self; + let mut retval = output.to_args(); + if let Some(file_stem) = file_stem { + retval.push(str::intern_owned(format!("--file-stem={file_stem}"))); + } + retval.push(str::intern(module_name)); + retval + } + pub fn file_with_ext(&self, ext: &str) -> String { + let mut retval = std::path::Path::new(self.output.path()) + .join(self.file_stem.as_ref().unwrap_or(&self.module_name)); + retval.set_extension(ext); + retval + .into_os_string() + .into_string() + .expect("known to be UTF-8") + } +} diff --git a/crates/fayalite/src/build/firrtl.rs b/crates/fayalite/src/build/firrtl.rs new file mode 100644 index 0000000..7c9998f --- /dev/null +++ b/crates/fayalite/src/build/firrtl.rs @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +// See Notices.txt for copyright information + +use crate::{ + build::{BaseArgs, DynJob, InternalJobTrait, JobItem, JobItemName}, + firrtl::{ExportOptions, FileBackend}, + intern::{Intern, Interned}, + util::job_server::AcquiredJob, +}; +use clap::Parser; +use std::{borrow::Cow, collections::BTreeMap}; + +#[derive(Parser, Debug, Clone, Hash, PartialEq, Eq)] +#[non_exhaustive] +pub struct FirrtlArgs { + #[command(flatten)] + pub base: BaseArgs, + #[command(flatten)] + pub export_options: ExportOptions, +} + +impl FirrtlArgs { + fn make_firrtl_file_backend(&self) -> FileBackend { + FileBackend { + dir_path: self.base.output.path().into(), + top_fir_file_stem: self.base.file_stem.clone(), + circuit_name: None, + } + } + pub fn firrtl_file(&self) -> String { + self.base.file_with_ext("fir") + } +} + +impl InternalJobTrait for FirrtlArgs { + fn subcommand_name() -> Interned { + "firrtl".intern() + } + + fn to_args(&self) -> Vec> { + let Self { + base, + export_options, + } = self; + let mut retval = base.to_args(); + retval.extend(export_options.to_args()); + retval + } + + fn inputs_and_direct_dependencies<'a>( + &'a self, + ) -> Cow<'a, BTreeMap>> { + Cow::Owned(BTreeMap::from_iter([( + JobItemName::Module { + name: str::intern(&self.base.module_name), + }, + None, + )])) + } + + fn outputs(&self) -> Interned<[JobItemName]> { + [JobItemName::File { + path: str::intern_owned(self.firrtl_file()), + }][..] + .intern() + } + + fn run( + &self, + inputs: &[JobItem], + _acquired_job: &mut AcquiredJob, + ) -> eyre::Result> { + let [JobItem::Module { value: module }] = inputs else { + panic!("wrong inputs, expected a single `Module`"); + }; + assert_eq!(*module.name(), *self.base.module_name); + crate::firrtl::export(self.make_firrtl_file_backend(), module, self.export_options)?; + Ok(vec![JobItem::File { + path: str::intern_owned(self.firrtl_file()), + }]) + } +} diff --git a/crates/fayalite/src/cli.rs b/crates/fayalite/src/cli.rs index 6fb4b5e..85095da 100644 --- a/crates/fayalite/src/cli.rs +++ b/crates/fayalite/src/cli.rs @@ -102,7 +102,7 @@ impl BaseArgs { mut captured_output: Option<&mut String>, ) -> io::Result { if self.redirect_output_for_rust_test || captured_output.is_some() { - let (reader, writer) = os_pipe::pipe()?; + let (reader, writer) = io::pipe()?; let mut reader = io::BufReader::new(reader); command.stderr(writer.try_clone()?); command.stdout(writer); // must not leave writer around after spawning child diff --git a/crates/fayalite/src/firrtl.rs b/crates/fayalite/src/firrtl.rs index b4a1d14..5fa6644 100644 --- a/crates/fayalite/src/firrtl.rs +++ b/crates/fayalite/src/firrtl.rs @@ -1206,9 +1206,7 @@ impl<'a> Exporter<'a> { | CanonicalType::AsyncReset(_) | CanonicalType::Reset(_) => Ok(format!("asUInt({value_str})")), CanonicalType::PhantomConst(_) => Ok("UInt<0>(0)".into()), - CanonicalType::DynSimOnly(_) => { - Err(FirrtlError::SimOnlyValuesAreNotPermitted.into()) - } + CanonicalType::DynSimOnly(_) => Err(FirrtlError::SimOnlyValuesAreNotPermitted.into()), } } fn expr_cast_bits_to_bundle( @@ -1429,9 +1427,7 @@ impl<'a> Exporter<'a> { definitions.add_definition_line(format_args!("{extra_indent}invalidate {retval}")); return Ok(retval.to_string()); } - CanonicalType::DynSimOnly(_) => { - Err(FirrtlError::SimOnlyValuesAreNotPermitted.into()) - } + CanonicalType::DynSimOnly(_) => Err(FirrtlError::SimOnlyValuesAreNotPermitted.into()), } } fn expr_unary( @@ -2758,7 +2754,7 @@ pub struct ExportOptions { #[clap(long = "no-simplify-memories", action = clap::ArgAction::SetFalse)] pub simplify_memories: bool, #[clap(long, value_parser = OptionSimplifyEnumsKindValueParser, default_value = "replace-with-bundle-of-uints")] - pub simplify_enums: std::option::Option, + pub simplify_enums: std::option::Option, // use std::option::Option instead of Option to avoid clap mis-parsing #[doc(hidden)] #[clap(skip = ExportOptionsPrivate(()))] /// `#[non_exhaustive]` except allowing struct update syntax @@ -2772,6 +2768,28 @@ impl fmt::Debug for ExportOptions { } impl ExportOptions { + pub fn to_args(&self) -> Vec> { + let Self { + simplify_memories, + simplify_enums, + __private: ExportOptionsPrivate(()), + } = self; + let mut retval = Vec::new(); + if !*simplify_memories { + retval.push("--no-simplify-memories".intern()); + } + let simplify_enums = simplify_enums.map(|v| { + clap::ValueEnum::to_possible_value(&v).expect("there are no skipped variants") + }); + let simplify_enums = match &simplify_enums { + None => OptionSimplifyEnumsKindValueParser::NONE_NAME, + Some(v) => v.get_name(), + }; + retval.push(str::intern_owned(format!( + "--simplify-enums={simplify_enums}" + ))); + retval + } fn debug_fmt( &self, f: &mut fmt::Formatter<'_>,