Skip to content

[core] Add Wire association support#5251

Merged
seldridge merged 3 commits into
mainfrom
dev/seldridge/domain-wire-association-opencode-qwen3.59b
Apr 1, 2026
Merged

[core] Add Wire association support#5251
seldridge merged 3 commits into
mainfrom
dev/seldridge/domain-wire-association-opencode-qwen3.59b

Conversation

@seldridge
Copy link
Copy Markdown
Member

@seldridge seldridge commented Mar 29, 2026

Extend the current associate API to also allow specifying domain associations on wires. This is necessary for certain libraries that eschew the existence of ports and use wires everywhere (e.g., diplomacy).

This introduces new FIRRTL syntax like the following to represent this:

wire w: UInt<1> domains [A, B]

This syntax is not currently supported, but an outstanding PR adds support for it [1]. In the interim, this commit includes an XFAIL test that will start passing once CIRCT support lands.

AI-assisted-by: pi.dev (Qwen 3.5)
AI-assisted-by: Claude Code (Claude Sonnet 4.6, Claude Haiku 4.5)

Release Notes

Extend associate API to allow adding domain associations to wires.

@seldridge seldridge requested a review from jackkoenig March 29, 2026 00:53
@seldridge seldridge added DO NOT MERGE No Release Notes Exclude from release notes, consider using Internal instead labels Mar 29, 2026
@seldridge seldridge force-pushed the dev/seldridge/domain-wire-association-opencode-qwen3.59b branch 2 times, most recently from 80e2e87 to 4b1cd61 Compare March 31, 2026 21:46
@seldridge seldridge marked this pull request as ready for review March 31, 2026 22:01
Extend the current `associate` API to also allow specifying domain
associations on wires.  This is necessary for certain libraries that
eschew the existence of ports and use wires everywhere (e.g., diplomacy).

This introduces new FIRRTL syntax like the following to represent this:

    wire w: UInt<1> domains [A, B]

This syntax is not currently supported, but an outstanding PR adds support
for it [[1]].  In the interim, this commit includes an XFAIL test that
will start passing once CIRCT support lands.

[1]: llvm/circt#10065

AI-assisted-by: pi.dev (Qwen 3.5)
AI-assisted-by: Claude Code (Claude Sonnet 4.6, Claude Haiku 4.5)
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
@seldridge seldridge force-pushed the dev/seldridge/domain-wire-association-opencode-qwen3.59b branch from 4b1cd61 to 671da0e Compare March 31, 2026 22:55
@seldridge seldridge added Feature New feature, will be included in release notes and removed DO NOT MERGE No Release Notes Exclude from release notes, consider using Internal instead labels Mar 31, 2026
@seldridge seldridge changed the title [core] Add Wire association support (Qwen3.59b) [core] Add Wire association support Mar 31, 2026
Comment thread core/src/main/scala/chisel3/internal/firrtl/Converter.scala Outdated
Comment thread core/src/main/scala/chisel3/Module.scala Outdated
}

/** Associate a port or wire of this module with one or more domains. */
final def associate(data: Data, domains: domain.Type*)(implicit si: SourceInfo): Unit = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think changing the name of this argument is right, but note that it is technically a source-incompatible change (since one can use argument names when calling functions). I'm not going to block it here but we should be careful with this generally.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do we have any guidance / policy around this?

Generally, source incompatible seems less bad as at least the compilation will error.

Is there a way to do this that wouldn't break source compatibility without going through a multi-stage deprecation process? I.e., I don't think we can have a duplicate definition with the same type signature.

Copy link
Copy Markdown
Contributor

@jackkoenig jackkoenig Apr 1, 2026

Choose a reason for hiding this comment

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

Generally, source incompatible seems less bad as at least the compilation will error.

I agree they're less bad but still something to avoid, especially on minor versions. This API is new (and in flux generally) enough that I think it's fine but if someone were to try changing the name of the argument to RegNext I would say no.

Is there a way to do this that wouldn't break source compatibility without going through a multi-stage deprecation process? I.e., I don't think we can have a duplicate definition with the same type signature.

You can two-step it with @deprecatedName. Again not necessary here.

Comment thread core/src/main/scala/chisel3/internal/firrtl/IR.scala Outdated
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
@seldridge seldridge requested a review from jackkoenig March 31, 2026 23:42
@seldridge seldridge merged commit d480263 into main Apr 1, 2026
28 checks passed
@seldridge seldridge deleted the dev/seldridge/domain-wire-association-opencode-qwen3.59b branch April 1, 2026 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature New feature, will be included in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants