Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derive abstract cells in top module during hierarchy #4930

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

KrystalDelusion
Copy link
Member

@KrystalDelusion KrystalDelusion commented Mar 6, 2025

What are the reasons/motivation for this change?

Previously if hierarchy had a top module then it would not derive any other abstract cells, this means that -check could miss certain kinds of errors that it would normally detect, such as in #4927.

Explain how this is achieved.

After finding (and potentially deriving) top_mod, find all of the cells included and derive them as well; similarly to how if there is no top module then all abstract modules are derived. I'm just now realising this only works for one level of abstract cells, which means we probably need some kind of recursive approach instead so I'm leaving this as draft for now. Also because it breaks under connect_rpc.

If applicable, please suggest to reviewers how they can test the change.

Includes tests/various/hierarchy_reassign_localparam.ys which fails under previous versions (instead giving the less helpful ERROR: Module name in defparam contains non-constant expressions!), but works when not using read_verilog without -defer, and/or when using hierarchy -check without -top.

i.e. Wrong error message mentioning defparam when overriding localparam #4927
Previously abstract cells would only be derived if there was no top module declared, this means that `-check` would miss certain kinds of errors that it would normally detect.  This fixes #4927.
@KrystalDelusion
Copy link
Member Author

I seem to have found a can of worms.

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.

Wrong error message mentioning defparam when overriding localparam
1 participant