Skip to content

Conversation

@seldridge
Copy link
Member

Extend domains to allow for them to contain fields. These fields provide
a schema of information that all domains of certain kind must have. This
models information that a user must provide for an input domain or is
provided for an output domain.

Opt for a "struct-like" representation as opposed to using input/output
ports like FIRRTL classes. That said, these are intended to be lowered to
classes in a future patch.

@seldridge seldridge requested a review from darthscsi as a code owner October 13, 2025 17:05
@seldridge seldridge requested review from dtzSiFive, dtzWill and rwy7 and removed request for dtzWill October 13, 2025 17:10
@seldridge seldridge force-pushed the dev/seldridge/domains-as-structs branch from 9365f0d to 84296d0 Compare October 13, 2025 17:25
Extend domains to allow for them to contain fields.  These fields provide
a schema of information that all domains of certain kind must have.  This
models information that a user must provide for an input domain or is
provided for an output domain.

Opt for a "struct-like" representation as opposed to using input/output
ports like FIRRTL classes.  That said, these are intended to be lowered to
classes in a future patch.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge force-pushed the dev/seldridge/domains-as-structs branch from 84296d0 to 984a444 Compare October 13, 2025 19:34
@seldridge seldridge merged commit 984a444 into main Oct 13, 2025
5 checks passed
@seldridge seldridge deleted the dev/seldridge/domains-as-structs branch October 13, 2025 19:38
@dtzSiFive
Copy link
Contributor

LGTM!
It might make sense to limit the field to a Property-type TypeAttr, ODS or verifier, since looks like at the FIRRTL text level that's an error to not be a property type.

If the field names must be unique (they ofc "must" if indexing is string-based?) consider verifying that as well.

Nice focused incremental addition!

@seldridge
Copy link
Member Author

It might make sense to limit the field to a Property-type TypeAttr, ODS or verifier, since looks like at the FIRRTL text level that's an error to not be a property type.

Good point. I started down this path and couldn't sort through the ODS / header inclusion to get it to compile.

If the field names must be unique (they ofc "must" if indexing is string-based?) consider verifying that as well.

Great idea. I'll add this in a follow-up.

@seldridge
Copy link
Member Author

@dtzSiFive wrote:

It might make sense to limit the field to a Property-type TypeAttr, ODS or verifier, since looks like at the FIRRTL text level that's an error to not be a property type.

PR here: #9094 The primary review request on #9094 is on the reorganization of the ODS files and headers.

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