Skip to content

Improve synthesized inter-module wiring and driver-name selection - #726

Merged
mkorbel1 merged 8 commits into
intel:mainfrom
mkorbel1:fix/submodule-output-bus-mapping
Oct 5, 2026
Merged

mkorbel1 merged 8 commits into
intel:mainfrom
mkorbel1:fix/submodule-output-bus-mapping

Conversation

@mkorbel1

@mkorbel1 mkorbel1 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Description & Motivation

Improve generated SystemVerilog for connections between modules, eliminating redundant intermediate signals and assignments where the complete connection can be represented directly at a port.

  • Map scalar child outputs directly into packed destination bits for assignSubset, swizzle, and rswizzle, including clearable alias chains and interior bits beside constants.
  • Connect fully covered, non-array packed child outputs directly to concatenations of their split destinations, including scalar signals and packed, unpacked, or mixed array elements. Array sources are not included in this transformation.
  • Remove obsolete custom-SystemVerilog output declarations after remapping, while retaining declarations needed by fanout or custom modules that require connected ports.
  • Omit redundant full-width scalar source selections while preserving true subranges, reversed bit order, and required array selections.
  • Prefer driver/output names over receiver/input names within equal-priority naming tiers. Explicit names, parent ports, preferred names, and name availability retain their existing precedence; this is a best-effort preference rather than a naming guarantee.

The additional synthesis cases also exposed two correctness issues addressed here: pruning could remove an inline slice driver still referenced by a synthesized range assignment, and preserved-name scalar Z constants could become undriven X in SystemVerilog. The fixes retain the required drivers without broadly retaining floating array helpers or changing net cleanup.

The optimizations retain guards for preserved names, additional readers, fanout, overlapping or incomplete coverage, constants, and net/inout boundaries. Cases outside the supported transformations keep their existing wiring.

Related Issue(s)

N/A.

Testing

Synthetic regressions check emitted declarations and port mappings, functional equivalence, bit order, partial destinations, constants and X/Z, naming stability across SV/netlist synthesis, custom module behavior, and 33/65-bit boundaries. Icarus is used for ordinary four-state HDL checks. Unpacked-array integration uses Verilator for two-state simulation and ROHD for four-state checks.

The initially exhaustive structural matrices were too expensive. Replace their 2,304 combinations with 424 targeted cases, preserving range/name/fanout interactions, producer variants, boundary slices, constant positions, and named-Z regressions. Each retained case uses 28 routing-focused vectors, with full 256-vector enumeration retained for seven representative cases. Dedicated safety and minimal bug regressions remain active.

Separate source checks, native VM and Node tests, and dev-container VM and Node tests into independent CI jobs. Keep the existing Run Checks status as an aggregate gate that fails when any constituent job fails, is cancelled, or is skipped. Test-platform splitting preserves the existing default runner behavior and all benchmark and platform checks; it does not merely raise the combined timeout. Stub-based shell tests cover platform selection and failure propagation.

The Verilator helper's expected missing-tool and warning self-tests capture their deliberate diagnostics, without suppressing unexpected compiler warnings or errors.

Backwards Compatibility

Design-construction APIs are unchanged. Generated SystemVerilog may contain fewer declarations, assignments, and redundant selections, and different canonical internal names. Downstream tooling that depends on exact generated text or internal signal names may need updates.

SimCompare.checkVerilatorVector adds an optional onSkip callback; its default still marks the current test skipped. tool/gh_actions/run_tests.sh accepts all, vm, or node; invocation without arguments still runs both platforms.

Documentation

Naming precedence is documented in the shared namer. Comments beside the test-matrix filters explain the retained representative coverage. No changes to design-authoring workflows are required.

@mkorbel1 mkorbel1 changed the title Eliminate redundant wires between submodule outputs and parent bus bits Improve synthesized inter-module wiring and driver-name selection Oct 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@mkorbel1
mkorbel1 requested review from desmonddak and a balanced review from Copilot October 5, 2026 20:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad synthesis-graph rewrites and pruning changes warrant final human validation despite extensive regression coverage.

Review effort: Balanced
Findings: None

Comment thread lib/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart Outdated

@desmonddak desmonddak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment as I walked through the optimizations and curious if they could be moved to synth. I do have some netlist vs SV code I use for validation, so I could look at this later along with a whole bunch of netlist optimizations which are too risky to push into synth without extensive corner case testing.

@mkorbel1
mkorbel1 merged commit 373a379 into intel:main Oct 5, 2026
13 checks passed
@mkorbel1
mkorbel1 deleted the fix/submodule-output-bus-mapping branch October 5, 2026 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants