Skip to content

simplify: Skip AST_PRIMITIVE in AST_CELLARRAY #4959

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KrystalDelusion
Copy link
Member

@KrystalDelusion KrystalDelusion commented Mar 24, 2025

What are the reasons/motivation for this change?

Close #4785.

We have an input error about cell arrays of primitives not being supported, but the AST_PRIMITIVE is simplified before it can be checked, leading to an assertion error instead.

Explain how this is achieved.

Don't simplify AST_PRIMITIVE in AST_CELLARRAY, similar to #3467.

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

Run yosys tests/verilog/bug4785.ys with/without this patch.

Otherwise the `AST_PRIMITIVE` simplifies to the corresponding function and is no longer caught by the check for `AST_PRIMITIVE`s, raising an assertion error instead of an input error.
Add bug4785.ys to tests/verilog to demonstrate.
@KrystalDelusion KrystalDelusion requested a review from zachjs as a code owner March 24, 2025 23:19
@KrystalDelusion
Copy link
Member Author

As pointed out in #4785, wrapping the primitive in a module, and then using an array of the wrapper module works fine. So it may be possible to implement support for cell arrays of primitives, but I'm not sure how much work that is. And there was already an error message for them being unsupported so 🤷 At least raising the proper error makes it clear we don't support it, rather than the mystery of an assertion error.

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.

read_verilog: array of instances parsing assertion failure
1 participant