From 32c94a53e13c7c8b6e93b5af6e97ead9af75c88f Mon Sep 17 00:00:00 2001 From: Tim Fischer Date: Thu, 2 Apr 2026 09:15:10 +0200 Subject: [PATCH 1/5] script: filter reachable files from top modules --- src/cmd/script.rs | 106 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 5 deletions(-) diff --git a/src/cmd/script.rs b/src/cmd/script.rs index f7fd4f58..b913ca05 100644 --- a/src/cmd/script.rs +++ b/src/cmd/script.rs @@ -3,6 +3,7 @@ //! The `script` subcommand. +use std::collections::HashSet; use std::io::Write; use std::path::{Path, PathBuf}; @@ -21,6 +22,9 @@ use crate::sess::{Session, SessionIo}; use crate::src::{SourceFile, SourceGroup, SourceType}; use crate::target::TargetSet; +#[cfg(feature = "slang")] +use bender_slang::SlangSession; + /// Emit tool scripts for the package #[derive(Args, Debug)] pub struct ScriptArgs { @@ -74,6 +78,11 @@ pub struct ScriptArgs { #[arg(long, global = true, help_heading = "General Script Options")] pub no_abort_on_error: bool, + /// One or more top-level modules used to trim unreachable source files. + #[cfg(feature = "slang")] + #[arg(long, global = true, help_heading = "General Script Options")] + pub top: Vec, + /// Format of the generated script #[command(subcommand)] pub format: ScriptFormat, @@ -235,9 +244,6 @@ pub fn run(sess: &Session, args: &ScriptArgs) -> Result<()> { // Format-specific target specifiers. let vivado_targets = &["vivado", "fpga", "xilinx"]; - fn concat(a: &[T], b: &[T]) -> Vec { - a.iter().chain(b).cloned().collect() - } let format_targets: Vec<&str> = if !args.no_default_target { match args.format { ScriptFormat::Flist { .. } => vec!["flist"], @@ -249,8 +255,8 @@ pub fn run(sess: &Session, args: &ScriptArgs) -> Result<()> { ScriptFormat::Formality => vec!["synopsys", "synthesis", "formality"], ScriptFormat::Riviera { .. } => vec!["riviera", "simulation"], ScriptFormat::Genus => vec!["genus", "synthesis"], - ScriptFormat::Vivado { .. } => concat(vivado_targets, &["synthesis"]), - ScriptFormat::VivadoSim { .. } => concat(vivado_targets, &["simulation"]), + ScriptFormat::Vivado { .. } => [vivado_targets as &[_], &["synthesis"]].concat(), + ScriptFormat::VivadoSim { .. } => [vivado_targets as &[_], &["simulation"]].concat(), ScriptFormat::Precision => vec!["precision", "fpga", "synthesis"], ScriptFormat::Template { .. } => vec![], ScriptFormat::TemplateJson => vec![], @@ -306,6 +312,14 @@ pub fn run(sess: &Session, args: &ScriptArgs) -> Result<()> { .map(|f| f.validate(&ValidationContext::default())) .collect::>>()?; + // Slang-based --top filtering: trim unreachable Verilog files. + #[cfg(feature = "slang")] + let srcs = if !args.top.is_empty() { + filter_srcs_by_top(srcs, &args.top)? + } else { + srcs + }; + let mut tera_context = Context::new(); let mut only_args = OnlyArgs { defines: false, @@ -420,6 +434,88 @@ where } } +/// Filter source groups to only include Verilog files reachable from the given top modules. +/// Non-Verilog files (VHDL, unknown) are always kept. Empty groups after filtering are dropped. +#[cfg(feature = "slang")] +fn filter_srcs_by_top<'a>( + srcs: Vec>, + top: &[String], +) -> Result>> { + use std::collections::HashMap; + + let mut session = SlangSession::new(); + let mut index_to_path: HashMap = HashMap::new(); + + for src_group in &srcs { + // Collect include dirs + let include_dirs: Vec = src_group + .include_dirs + .iter() + .chain(src_group.export_incdirs.values().flatten()) + .map(|(_, path)| path.to_string_lossy().into_owned()) + .collect::>() + .into_iter() + .collect(); + + // Collect defines + let defines: Vec = src_group + .defines + .iter() + .map(|(def, (_, value))| match value { + Some(v) => format!("{def}={v}"), + None => def.to_string(), + }) + .collect::>() + .into_iter() + .collect(); + + // Collect only Verilog file paths. + let paths: Vec<&Path> = src_group + .files + .iter() + .filter_map(|f| match f { + SourceFile::File(p, Some(SourceType::Verilog)) => Some(*p), + _ => None, + }) + .collect(); + + if !paths.is_empty() { + let file_paths: Vec = paths + .iter() + .map(|p| p.to_string_lossy().into_owned()) + .collect(); + let indices = session + .parse_group(&file_paths, &include_dirs, &defines) + .into_diagnostic()?; + for (idx, path) in indices.into_iter().zip(&paths) { + index_to_path.insert(idx, path); + } + } + } + + // Get the indices of Verilog files reachable from the top modules. + let reachable_indices = session.reachable_indices(top).into_diagnostic()?; + // Map the reachable indices back to paths and collect them in a set for easy lookup. + let kept_paths: HashSet<&Path> = reachable_indices + .iter() + .filter_map(|i| index_to_path.get(i).copied()) + .collect(); + + Ok(srcs + .into_iter() + // For each source group, retain only the Verilog files that are in the set of reachable paths. + .map(|mut group| { + group.files.retain(|f| match f { + SourceFile::File(p, Some(SourceType::Verilog)) => kept_paths.contains(p), + _ => true, + }); + group + }) + // Remove empty groups that may have resulted from filtering out all Verilog files. + .filter(|group| !group.files.is_empty()) + .collect()) +} + static HEADER_AUTOGEN: &str = "This script was generated automatically by bender."; fn add_defines(defines: &mut IndexMap>, define_args: &[String]) { From 83d3acee6baabad73478fc5f9312d6705e8b55b9 Mon Sep 17 00:00:00 2001 From: Tim Fischer Date: Thu, 2 Apr 2026 09:17:27 +0200 Subject: [PATCH 2/5] bender-slang: implement last-wins semantics for duplicate definitions --- crates/bender-slang/cpp/analysis.cpp | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/bender-slang/cpp/analysis.cpp b/crates/bender-slang/cpp/analysis.cpp index c1989b5d..bfd87ae0 100644 --- a/crates/bender-slang/cpp/analysis.cpp +++ b/crates/bender-slang/cpp/analysis.cpp @@ -4,7 +4,9 @@ #include "slang_bridge.h" #include +#include #include +#include #include #include @@ -13,13 +15,21 @@ using namespace slang; rust::Vec reachable_tree_indices(const SlangSession& session, const rust::Vec& tops) { const auto& treeVec = session.trees(); - // Build a mapping from declared symbol names to the index of the tree that - // declares them. + // Build the name-to-tree-index map with last-wins semantics, emitting a warning + // whenever a later definition overwrites an earlier one. std::unordered_map nameToTreeIndex; for (size_t i = 0; i < treeVec.size(); ++i) { - const auto& metadata = treeVec[i]->getMetadata(); - for (auto name : metadata.getDeclaredSymbols()) { - nameToTreeIndex.emplace(name, i); + for (auto name : treeVec[i]->getMetadata().getDeclaredSymbols()) { + auto [it, inserted] = nameToTreeIndex.emplace(name, i); + if (!inserted) { + std::cerr << "warning[BND]: Module '" << name << "' defined in " + << treeVec[i]->sourceManager().getRawFileName(treeVec[i]->getSourceBufferIds()[0]) + << " overwrites previous definition in " + << treeVec[it->second]->sourceManager().getRawFileName( + treeVec[it->second]->getSourceBufferIds()[0]) + << ".\n"; + it->second = i; + } } } From 401e73dc1e9ffc6bb23192258316a2f7b138764e Mon Sep 17 00:00:00 2001 From: Tim Fischer Date: Thu, 2 Apr 2026 09:17:49 +0200 Subject: [PATCH 3/5] tests: add duplicate module test --- tests/pickle/Bender.yml | 6 +++ tests/pickle/src/dup_a.sv | 3 ++ tests/pickle/src/dup_b.sv | 3 ++ tests/pickle/src/dup_top.sv | 3 ++ tests/script.rs | 91 +++++++++++++++++++++++++++++++++++++ 5 files changed, 106 insertions(+) create mode 100644 tests/pickle/src/dup_a.sv create mode 100644 tests/pickle/src/dup_b.sv create mode 100644 tests/pickle/src/dup_top.sv create mode 100644 tests/script.rs diff --git a/tests/pickle/Bender.yml b/tests/pickle/Bender.yml index 7373ef37..ed070503 100644 --- a/tests/pickle/Bender.yml +++ b/tests/pickle/Bender.yml @@ -18,3 +18,9 @@ sources: - include files: - src/top.sv + + - target: dup + files: + - src/dup_a.sv + - src/dup_b.sv + - src/dup_top.sv diff --git a/tests/pickle/src/dup_a.sv b/tests/pickle/src/dup_a.sv new file mode 100644 index 00000000..08715784 --- /dev/null +++ b/tests/pickle/src/dup_a.sv @@ -0,0 +1,3 @@ +// Version A of dup_mod (e.g. from a dependency package) +module dup_mod #(parameter int VERSION = 1); +endmodule diff --git a/tests/pickle/src/dup_b.sv b/tests/pickle/src/dup_b.sv new file mode 100644 index 00000000..530053a6 --- /dev/null +++ b/tests/pickle/src/dup_b.sv @@ -0,0 +1,3 @@ +// Version B of dup_mod (e.g. a project-level override of the dependency) +module dup_mod #(parameter int VERSION = 2); +endmodule diff --git a/tests/pickle/src/dup_top.sv b/tests/pickle/src/dup_top.sv new file mode 100644 index 00000000..f37ac658 --- /dev/null +++ b/tests/pickle/src/dup_top.sv @@ -0,0 +1,3 @@ +module dup_top; + dup_mod u(); +endmodule diff --git a/tests/script.rs b/tests/script.rs new file mode 100644 index 00000000..94bc7a9f --- /dev/null +++ b/tests/script.rs @@ -0,0 +1,91 @@ +// Copyright (c) 2025 ETH Zurich +// Tim Fischer + +#[cfg(feature = "slang")] +mod tests { + use assert_cmd::cargo; + + fn run_script(args: &[&str]) -> String { + let mut full_args = vec!["-d", "tests/pickle", "script"]; + full_args.extend(args); + + let out = cargo::cargo_bin_cmd!() + .args(&full_args) + .output() + .expect("Failed to execute bender binary"); + + assert!( + out.status.success(), + "script command failed.\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr) + ); + + String::from_utf8(out.stdout).expect("stdout must be utf-8") + } + + #[test] + fn script_top_filters_unreachable_files() { + // Without --top: all files present + let full = run_script(&["--target", "top", "flist-plus"]); + assert!(full.contains("unused_top.sv")); + assert!(full.contains("unused_leaf.sv")); + + // With --top top: unreachable files removed + let trimmed = run_script(&["--target", "top", "--top", "top", "flist-plus"]); + assert!(trimmed.contains("top.sv")); + assert!(trimmed.contains("core.sv")); + assert!(trimmed.contains("leaf.sv")); + assert!(!trimmed.contains("unused_top.sv")); + assert!(!trimmed.contains("unused_leaf.sv")); + } + + #[test] + fn script_top_multiple_tops() { + let trimmed = run_script(&[ + "--target", + "top", + "--top", + "top", + "--top", + "unused_top", + "flist-plus", + ]); + assert!(trimmed.contains("top.sv")); + assert!(trimmed.contains("unused_top.sv")); + } + + #[test] + fn script_top_empty_keeps_all_files() { + // Without --top: all files appear + let full = run_script(&["--target", "top", "flist-plus"]); + assert!(full.contains("top.sv")); + assert!(full.contains("core.sv")); + assert!(full.contains("leaf.sv")); + assert!(full.contains("unused_top.sv")); + assert!(full.contains("unused_leaf.sv")); + } + + /// Regression test: when two files define the same module name, last-wins semantics apply. + /// The file parsed last (dup_b.sv) wins; the earlier definition (dup_a.sv) is dropped. + #[test] + fn script_top_duplicate_module_name_last_wins() { + // Without --top: both dup files appear (no filtering applied) + let full = run_script(&["--target", "dup", "flist-plus"]); + assert!(full.contains("dup_a.sv")); + assert!(full.contains("dup_b.sv")); + assert!(full.contains("dup_top.sv")); + + // With --top dup_top: only dup_b.sv (last-wins) and dup_top.sv appear + let trimmed = run_script(&["--target", "dup", "--top", "dup_top", "flist-plus"]); + assert!(trimmed.contains("dup_top.sv")); + assert!( + trimmed.contains("dup_b.sv"), + "dup_b.sv (last-wins) missing:\n{trimmed}" + ); + assert!( + !trimmed.contains("dup_a.sv"), + "dup_a.sv (overwritten) should be absent:\n{trimmed}" + ); + } +} From f18c6ff731ccc90ac97c984c56bcfed98df3348c Mon Sep 17 00:00:00 2001 From: Tim Fischer Date: Thu, 2 Apr 2026 12:41:09 +0200 Subject: [PATCH 4/5] bender-slang: emit slang style warnings for duplicate declarations --- crates/bender-slang/cpp/analysis.cpp | 35 +++++++++++++++++++++------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/bender-slang/cpp/analysis.cpp b/crates/bender-slang/cpp/analysis.cpp index bfd87ae0..9746fae7 100644 --- a/crates/bender-slang/cpp/analysis.cpp +++ b/crates/bender-slang/cpp/analysis.cpp @@ -1,12 +1,14 @@ // Copyright (c) 2025 ETH Zurich // Tim Fischer +#include "slang/syntax/AllSyntax.h" #include "slang_bridge.h" #include #include #include #include +#include #include #include @@ -17,20 +19,37 @@ rust::Vec reachable_tree_indices(const SlangSession& session, con // Build the name-to-tree-index map with last-wins semantics, emitting a warning // whenever a later definition overwrites an earlier one. + slang::DiagCode overwriteCode(slang::DiagSubsystem::General, 9999); std::unordered_map nameToTreeIndex; for (size_t i = 0; i < treeVec.size(); ++i) { - for (auto name : treeVec[i]->getMetadata().getDeclaredSymbols()) { + const auto& metadata = treeVec[i]->getMetadata(); + + auto checkAndInsert = [&](std::string_view name, slang::SourceLocation loc) { + if (name.empty()) + return; auto [it, inserted] = nameToTreeIndex.emplace(name, i); if (!inserted) { - std::cerr << "warning[BND]: Module '" << name << "' defined in " - << treeVec[i]->sourceManager().getRawFileName(treeVec[i]->getSourceBufferIds()[0]) - << " overwrites previous definition in " - << treeVec[it->second]->sourceManager().getRawFileName( - treeVec[it->second]->getSourceBufferIds()[0]) - << ".\n"; + slang::DiagnosticEngine engine(treeVec[i]->sourceManager()); + auto client = std::make_shared(); + client->showColors(isatty(STDERR_FILENO)); + engine.addClient(client); + engine.setMessage(overwriteCode, "module '{}' overwrites previous definition in '{}'"); + engine.setSeverity(overwriteCode, slang::DiagnosticSeverity::Warning); + + slang::Diagnostic diag(overwriteCode, loc); + diag << name; + diag << treeVec[it->second]->sourceManager().getRawFileName( + treeVec[it->second]->getSourceBufferIds()[0]); + engine.issue(diag); + std::cerr << client->getString(); it->second = i; } - } + }; + + for (const auto& [decl, _] : metadata.nodeMeta) + checkAndInsert(decl->header->name.valueText(), decl->header->name.location()); + for (const auto classDecl : metadata.classDecls) + checkAndInsert(classDecl->name.valueText(), classDecl->name.location()); } // Build a dependency graph where each tree points to the trees that declare From c36b891b1c4195faf5cba65c0af5b3d84f7980b4 Mon Sep 17 00:00:00 2001 From: Tim Fischer Date: Thu, 2 Apr 2026 15:19:40 +0200 Subject: [PATCH 5/5] bender-slang: differentiate windows/unix TTY --- crates/bender-slang/cpp/analysis.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/bender-slang/cpp/analysis.cpp b/crates/bender-slang/cpp/analysis.cpp index 9746fae7..3cfc705b 100644 --- a/crates/bender-slang/cpp/analysis.cpp +++ b/crates/bender-slang/cpp/analysis.cpp @@ -8,7 +8,13 @@ #include #include #include +#ifdef _WIN32 +#include +#define STDERR_IS_TTY _isatty(_fileno(stderr)) +#else #include +#define STDERR_IS_TTY isatty(STDERR_FILENO) +#endif #include #include @@ -31,7 +37,7 @@ rust::Vec reachable_tree_indices(const SlangSession& session, con if (!inserted) { slang::DiagnosticEngine engine(treeVec[i]->sourceManager()); auto client = std::make_shared(); - client->showColors(isatty(STDERR_FILENO)); + client->showColors(STDERR_IS_TTY); engine.addClient(client); engine.setMessage(overwriteCode, "module '{}' overwrites previous definition in '{}'"); engine.setSeverity(overwriteCode, slang::DiagnosticSeverity::Warning);