Skip to content

Conversation

@rwy7
Copy link
Contributor

@rwy7 rwy7 commented Oct 9, 2025

Add checking to ensure that the domain information on instances lines up with what is recorded on the referenced module.

@rwy7 rwy7 requested a review from dtzSiFive October 9, 2025 21:20
@rwy7 rwy7 force-pushed the fix-domain-verifiers branch from 2135596 to 88d4508 Compare October 9, 2025 21:21
Copy link
Contributor

@dtzSiFive dtzSiFive left a comment

Choose a reason for hiding this comment

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

Verifier and test changes LGTM.

The builder changes -- they seem part of a related/necessary (but not in PR scope per title?) change: adding Domain support for InstanceChoiceOp's.

Please either mention this change in the title/commit or split out 👍.

(and consider positive test for instance choice op domain support?)

@rwy7 rwy7 force-pushed the fix-domain-verifiers branch 3 times, most recently from 0802c31 to a086f90 Compare November 4, 2025 20:19
@rwy7 rwy7 changed the title [FIRRTL] Add domain info verification to instance ops [FIRRTL] Add domain verification to instance ops and domain support to instance choice ops Nov 4, 2025
@rwy7 rwy7 force-pushed the fix-domain-verifiers branch from a086f90 to 2c8cfd9 Compare November 6, 2025 15:38
@rwy7
Copy link
Contributor Author

rwy7 commented Nov 6, 2025

pulled instance choice domain support out to its own PR: #9197

@rwy7 rwy7 force-pushed the fix-domain-verifiers branch from 2c8cfd9 to 8a75ea3 Compare November 6, 2025 17:41
@rwy7 rwy7 changed the title [FIRRTL] Add domain verification to instance ops and domain support to instance choice ops [FIRRTL] Add domain verification to instance-like ops Nov 6, 2025
@rwy7 rwy7 merged commit 83b6a1c into llvm:main Nov 6, 2025
7 checks passed
@rwy7 rwy7 deleted the fix-domain-verifiers branch November 6, 2025 18:01
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