Skip to content

Element, subset, and swizzle collapsing in generated SV#663

Open
mkorbel1 wants to merge 15 commits into
intel:mainfrom
mkorbel1:array_element_collapse
Open

Element, subset, and swizzle collapsing in generated SV#663
mkorbel1 wants to merge 15 commits into
intel:mainfrom
mkorbel1:array_element_collapse

Conversation

@mkorbel1

@mkorbel1 mkorbel1 commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Description & Motivation

This PR dramatically improves SV generation by reducing the amount of unnecessary assignments, net_connects, swizzles, subsets, and intermediate signals for Logics and LogicArrays (both net and non-net) in generated SystemVerilog. This makes the SV much more readable, debuggable, and tool friendly.

Related Issue(s)

N/A

Testing

Added extensive new testing for collapsing capabilities, existing suite helps protect.

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

No

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

No

@mkorbel1 mkorbel1 marked this pull request as ready for review July 1, 2026 17:35
Comment thread lib/src/synthesizers/systemverilog/systemverilog_synth_module_definition.dart Outdated
SimCompare.checkIverilogVector(mod, vectors);
});
}
});

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.

I didn't see any naming checks to see if you end up retaining only basename nets or accidentally toss that net and keep one with _0. I think this code is not enforcing the namig order, but it would be good to have a test for it and know how you are going to enforce better names (with a current test that you waive).

await SimCompare.checkFunctionalVector(mod, vectors);
SimCompare.checkIverilogVector(mod, vectors);
});
}

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.

I test is needed where a SynthLogic has been merged before a specific aggregate collapse happens.
element <= intermediate;
intermediate <= realSource;
You want to be sure you don't see the intermediate kept.

// --- functional correctness (always) ---
await SimCompare.checkFunctionalVector(mod, vectors);
SimCompare.checkIverilogVector(mod, vectors);
});

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.

Need a test showing a partial assignement is not collapsed.

@desmonddak

Copy link
Copy Markdown
Contributor

I think it makes sense to commit this PR for cleaning up the major cases. Two main arch points to prepare for is the naming problem, which we tried to solve in central_naming by marking first in the synth base classes, and then another is to think about what will be common optimizations versus SV specific.

Unfortunately a netlist branch is not yet in and that has a really general approach to deal with nested assignments and series of pack/unpack/pack as well as subset/concat/subset which can occur in a nested fashion (a very wide interface split into subsets and then recombined in a couple levels possibly with some splits and recombines and finally end up at the very same interface. Then mixed array and struct assignments, reversals, etc. all get very tricky, especially with partial assignments and subset assignments. Worth thinking about a general optimization approach along with the naming issue. Nested Interface assignments containing busses is the source of these.

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.

2 participants