Skip to content

Add support for array-typed parameters to VAST.#3685

Open
meheff wants to merge 2 commits intogoogle:mainfrom
xlsynth:meheff/2026-01-13-unpacked-arrays
Open

Add support for array-typed parameters to VAST.#3685
meheff wants to merge 2 commits intogoogle:mainfrom
xlsynth:meheff/2026-01-13-unpacked-arrays

Conversation

@meheff
Copy link
Collaborator

@meheff meheff commented Jan 14, 2026

These were representable but were unusable because they could not be converted to a IndexableExpression. Also add unpacked array parameters and array assignment to the C API.

meheff added a commit to xlsynth/xlsynth that referenced this pull request Jan 14, 2026
These were representable but were unusable because they could not be converted to a IndexableExpression. Also add this capability to the C API.

EXPECT_EQ(m->Emit(nullptr),
R"(module top;
parameter P0[2] = '{'0, '0};
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't believe it's valid SystemVerilog to have an unpacked param without an explicit data type. Similar below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the review. Sorry I missed it earlier.

There is not a specific example of this construct in the spec, but the grammar production rules allow it (6.20.1) and it doesn't appear to be explicitly disallowed. There is some discussion about aggregate constants at the end of 6.20.2 where such an exception seemingly would be made. I assume it behaves just as an array of the implicit data type (32-bit signed logic in SV).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Both Verific and VCS report syntax errors in this case. E.g.

Error-[SE] Syntax error
  Following verilog source has syntax error :
...
token is '='
    parameter P0[2] = '{'0, '0};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good. I removed them from the tests. I didn't add a check as checking is awkward as it would require deep inspection of the datatype and VAST generally does not do extensive checking of the AST for legality, just trivial checks.

cdleary added a commit to xlsynth/xlsynth that referenced this pull request Jan 30, 2026
These were representable but were unusable because they could not be converted to a IndexableExpression. Also add this capability to the C API.
@meheff meheff force-pushed the meheff/2026-01-13-unpacked-arrays branch from 6ed274b to 4152ca1 Compare February 3, 2026 18:35
@ericastor
Copy link
Contributor

On import, we're seeing unused variable warnings (which we treat as errors):

xls/codegen/vast/vast_test.cc:2138:15: error: unused variable 'tick0' [-Werror,-Wunused-variable]
 2138 |   Expression* tick0 = f.Make<UnsizedZeroLiteral>(si);
      |               ^~~~~
xls/codegen/vast/vast_test.cc:2157:15: error: unused variable 'p3_row' [-Werror,-Wunused-variable]
 2157 |   Expression* p3_row =
      |               ^~~~~~

Mind fixing this, or should we do it at point-of-import?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants