[core] Add initial domain support #5041

seldridge · 2025-10-01T21:28:10Z

Add support for creating domains and domain types. Add a built-in
ClockDomain type.

This is very basic support initially and many things will not work, namely
connections or compilation of these to anything meaningful.

Release Notes

Add initial support for creating domains. These are intended to model things like clock, reset, and power domains and their associations to ports in the design.

Extend the allowed binary incompatibility for more problems in `chisel3.internal.firrtl.ir*` and the same set of problems in `firrtl.ir*`. This is in support of allowing breakages that are necessary to represent domain associations. Signed-off-by: Schuyler Eldridge <[email protected]>

Add support for creating domains and domain types. Add a built-in ClockDomain type. This is very basic support initially and many things will not work, namely connections or compilation of these to anything meaningful. Signed-off-by: Schuyler Eldridge <[email protected]>

jackkoenig · 2025-10-16T15:54:39Z

core/src/main/scala/chisel3/domain/Domain.scala

+abstract class Domain()(implicit val sourceInfo: SourceInfo) { self: Singleton =>
+
+  // The name that will be used when generating FIRRTL.
+  private[chisel3] def name: String = simpleClassName(this.getClass())


Suggested change

private[chisel3] def name: String = simpleClassName(this.getClass())

private[chisel3] def name: String = simpleClassName(this.getClass)

Nit but we typically call .getClass without ().

jackkoenig · 2025-10-16T16:39:14Z

core/src/main/scala/chisel3/Module.scala

+    protected[chisel3] def getAssociations: scala.collection.immutable.Map[Data, LinkedHashSet[Data]] =
+      _associations.toMap


Is this really needed or can we just use _associations?

jackkoenig · 2025-10-16T17:36:28Z

core/src/main/scala/chisel3/Module.scala

+        Builder.error(s"""Unable to associate port '$port' to domains '${allDomains.mkString(
+            ", "
+          )}' because the port does not exist in this module""")(si)
+        println(getModulePorts)


Suggested change

println(getModulePorts)

stray println

jackkoenig · 2025-10-16T17:37:05Z

core/src/main/scala/chisel3/Module.scala

+        Builder.error(s"""Unable to associate port '$port' to domains '${allDomains.mkString(
+            ", "
+          )}' because the port does not exist in this module""")(si)


This is getting formatted weirdly, can you massage it a bit, e.g. assign the result of allDomains.mkString to a val.

jackkoenig · 2025-10-16T17:39:10Z

core/src/main/scala/chisel3/internal/firrtl/Serializer.scala

+  private def legalize(name: String): String = {
+    name match {
+      // If the name starts with a digit, then escape it with backticks.
+      case _ if name.head.isDigit => s"`$name`"
+      case _                      => name
+    }
  }


This change feels pointless but also is fine

jackkoenig · 2025-10-16T18:12:33Z

core/src/main/scala/chisel3/Module.scala

      require(_closed, "Can't get ports before module close")
-      _ports.toSeq
+      _ports.toSeq.map { case (port, info) =>
+        (port, info, _associations.get(port).map(_.toSeq).getOrElse(Seq.empty[Data]))


Suggested change

(port, info, _associations.get(port).map(_.toSeq).getOrElse(Seq.empty[Data]))

(port, info, _associations.get(port).fold(Seq.empty[Data])(_.toSeq)

I personally have taken to .fold(default)(f) instead of .map(f).getOrElse(default) but I'll defer to you on which you think is better.

jackkoenig · 2025-10-16T18:16:14Z

core/src/main/scala/chisel3/Module.scala

    protected def portsSize: Int = _ports.size

+    /* Associate a port of this module with one or more domains. */
+    final def associate(port: Data, domain: Data, domains: Data*)(implicit si: SourceInfo): Unit = {


So we've often used this T, T* as a trick to ensure one or more, but it makes certain things really awkward, e.g. trying to work with a Seq, you have to do:

associate(port, myDomains(0), myDomains.tail: _*)

I think it would be better to overload with:

final def associate(port: Data, domain: Data)(implicit si: SourceInfo): Unit = associate(port, Seq(domain)) final def associate(port: Data, domains: Seq[Data])(implicit si: SourceInfo): Unit = ...

Or just have a single version with varags and accept it's a runtime error if called with no domains?

Or I guess you could overload with

final def associate(port: Data, domain: Data, domains: Data*)(implicit si: SourceInfo): Unit = associate(port, Seq(domain) ++ domains) final def associate(port: Data, domains: Seq[Data])(implicit si: SourceInfo): Unit = ...

jackkoenig · 2025-10-16T18:17:06Z

core/src/main/scala/chisel3/domain/Type.scala

+/** A [[Data]] that is used to communicate information of a specific domain
+  * kind.
+  */
+final class Type private (val domain: Domain) extends Element { self =>


Does a domain.Type really need to be an Element or Data at all?

They do need to be ports, so I think it would either be changing the type that IO accepts or adding a new IO creation method just for domain.Type. Either of these is doable.

The more critical thing would be that I expect libraries that assume everything in Chisel can be shoved into a Wire, like diplomacy, will want to put domain.Type into a wire. We might be able to avoid this with changes to libraries so that they don't use a single Wire. (This is related to what I was talking to you offline about assuming everything can go into a Bundle is making things hard.) WDYT?

Ah I see, then I think Element is right. If these need to be ports and wired through then I think they run into the same needs as other things like Properties. As in the larger conversation about Bundles, maybe there's a better way out there but there has to be some way to compose these things together and Bundle (well Record) remains the only existing avenue.

jackkoenig · 2025-10-16T22:37:04Z

src/test/scala-2/chiselTests/DomainSpec.scala

+      val A = IO(Input(domain.Type(ClockDomain)))
+      val B = IO(Input(domain.Type(PowerDomain)))


This feels a bit inconsistent with the rest of Chisel, what about:

Suggested change

val A = IO(Input(domain.Type(ClockDomain)))

val B = IO(Input(domain.Type(PowerDomain)))

val A = IO(Input(ClockDomain()))

val B = IO(Input(PowerDomain()))

or perhaps

Suggested change

val A = IO(Input(domain.Type(ClockDomain)))

val B = IO(Input(domain.Type(PowerDomain)))

val A = IO(Input(ClockDomain.Type()))

val B = IO(Input(PowerDomain.Type()))

seldridge added the Feature New feature, will be included in release notes label Oct 1, 2025

seldridge force-pushed the dev/seldridge/domains branch 2 times, most recently from ce1f528 to 74f1910 Compare October 15, 2025 20:14

seldridge force-pushed the dev/seldridge/domains branch from 74f1910 to e40332d Compare October 15, 2025 20:28

seldridge force-pushed the dev/seldridge/domains branch from e40332d to 92af846 Compare October 15, 2025 21:04

seldridge marked this pull request as ready for review October 15, 2025 21:04

seldridge requested a review from jackkoenig October 15, 2025 21:04

fixup! [core] Add initial domain support

dea0bda

jackkoenig reviewed Oct 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[core] Add initial domain support #5041

[core] Add initial domain support #5041

Uh oh!

seldridge commented Oct 1, 2025 •

edited

Loading

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

seldridge Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

jackkoenig Oct 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	private[chisel3] def name: String = simpleClassName(this.getClass())
	private[chisel3] def name: String = simpleClassName(this.getClass)

		protected[chisel3] def getAssociations: scala.collection.immutable.Map[Data, LinkedHashSet[Data]] =
		_associations.toMap

	(port, info, _associations.get(port).map(_.toSeq).getOrElse(Seq.empty[Data]))
	(port, info, _associations.get(port).fold(Seq.empty[Data])(_.toSeq)

		val A = IO(Input(domain.Type(ClockDomain)))
		val B = IO(Input(domain.Type(PowerDomain)))

[core] Add initial domain support #5041

Are you sure you want to change the base?

[core] Add initial domain support #5041

Uh oh!

Conversation

seldridge commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

seldridge commented Oct 1, 2025 •

edited

Loading