From 27312ac360d27b1d7b646eefc8b2c1c4abd6dd25 Mon Sep 17 00:00:00 2001 From: Jack Koenig Date: Wed, 8 Apr 2026 20:09:23 -0700 Subject: [PATCH] Add .asProducer, .asConsumer and deprecated forms AI-assisted-by: Claude Code (Claude Opus 4.8) --- core/src/main/scala/chisel3/Data.scala | 63 ++- .../chisel3/connectable/Connection.scala | 103 ++++- .../experimental/dataview/package.scala | 88 +++- .../main/scala/chisel3/internal/Binding.scala | 16 +- .../main/scala/chisel3/internal/Warning.scala | 2 + docs/src/explanations/connectable.md | 118 ++++- docs/src/explanations/warnings.md | 97 ++++ release.mill | 7 +- .../chiselTests/AsProducerConsumerSpec.scala | 436 ++++++++++++++++++ 9 files changed, 869 insertions(+), 61 deletions(-) create mode 100644 src/test/scala/chiselTests/AsProducerConsumerSpec.scala diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala index 37719b77095..06095f1ab52 100644 --- a/core/src/main/scala/chisel3/Data.scala +++ b/core/src/main/scala/chisel3/Data.scala @@ -1070,14 +1070,61 @@ object Data { * It is illegal to connect to the return value of this method. * This Data this method is called on must be a hardware type. */ - def readOnly(implicit sourceInfo: SourceInfo): T = { - val alreadyReadOnly = self.isLit || self.topBindingOpt.exists(_.isInstanceOf[ReadOnlyBinding]) - if (alreadyReadOnly) { - self - } else { - self.viewAsReadOnly(_ => "Cannot connect to read-only value") - } - } + def readOnly(implicit sourceInfo: SourceInfo): T = + viewOrSelf(self)(self.viewAsReadOnly(_ => "Cannot connect to read-only value")) + } + + /** Returns `self` unchanged if it is already read-only (a literal or a [[ReadOnlyBinding]]), + * otherwise returns the result of `mk` (the read-only view to build). + */ + private def viewOrSelf[T <: Data](self: T)(mk: => T): T = + if (self.isLit || self.topBindingOpt.exists(_.isInstanceOf[ReadOnlyBinding])) self else mk + + implicit class AsProducerConsumer[T <: Data](self: T) { + + /** Returns a view of this Data marked as a producer. + * + * Aligned fields (outputs from the producer's perspective) become read-only, + * while flipped fields (inputs to the producer) remain writable. + * + * This should only be used on the producer (RHS) of Connectable operators + * (e.g., `consumer :<>= producer.asProducer`). + */ + def asProducer(implicit sourceInfo: SourceInfo): T = + viewOrSelf(self)(self.viewAsProducer(_ => "Cannot connect to producer's aligned field")) + + /** Returns a view of this Data marked as a consumer. + * + * Flipped fields (inputs from the consumer's perspective) become read-only, + * while aligned fields (outputs from the consumer) remain writable. + * + * This should only be used on the consumer (LHS) of Connectable operators + * (e.g., `consumer.asConsumer :<>= producer`). + */ + def asConsumer(implicit sourceInfo: SourceInfo): T = + viewOrSelf(self)(self.viewAsConsumer(_ => "Cannot connect to consumer's flipped field")) + + /** Like [[asProducer]] but issues deprecation warnings instead of errors. + * + * Use this to migrate existing code toward [[asProducer]] incrementally. + */ + def asProducerDeprecated(implicit sourceInfo: SourceInfo): T = + viewOrSelf(self)( + self.viewAsProducerDeprecated(info => + Warning(WarningID.AsProducerDeprecated, "Cannot connect to producer's aligned field")(info) + ) + ) + + /** Like [[asConsumer]] but issues deprecation warnings instead of errors. + * + * Use this to migrate existing code toward [[asConsumer]] incrementally. + */ + def asConsumerDeprecated(implicit sourceInfo: SourceInfo): T = + viewOrSelf(self)( + self.viewAsConsumerDeprecated(info => + Warning(WarningID.AsConsumerDeprecated, "Cannot connect to consumer's flipped field")(info) + ) + ) } } diff --git a/core/src/main/scala/chisel3/connectable/Connection.scala b/core/src/main/scala/chisel3/connectable/Connection.scala index 11bbf7cd654..5c2f18a85e0 100644 --- a/core/src/main/scala/chisel3/connectable/Connection.scala +++ b/core/src/main/scala/chisel3/connectable/Connection.scala @@ -3,8 +3,9 @@ package chisel3.connectable import chisel3.{Aggregate, BiConnectException, Data, DontCare, InternalErrorException, RawModule} -import chisel3.internal.{BiConnect, Builder} +import chisel3.internal.{BiConnect, Builder, Warning, WarningID} import chisel3.internal.Builder.pushCommand +import chisel3.internal.binding._ import chisel3.internal.firrtl.ir.DefInvalid import chisel3.experimental.{prefix, SourceInfo, UnlocatableSourceInfo} import chisel3.experimental.{attach, Analog} @@ -57,20 +58,27 @@ private[chisel3] case object ColonLessGreaterEq extends Connection { val connectToProducer: Boolean = true val alwaysConnectToConsumer: Boolean = false def canFirrtlConnect(consumer: Connectable[Data], producer: Connectable[Data]) = { - val typeEquivalent = - try { - BiConnect.canFirrtlConnectData( - consumer.base, - producer.base, - UnlocatableSourceInfo, - Builder.referenceUserModule - ) && consumer.base.typeEquivalent(producer.base) - } catch { - // For some reason, an error is thrown if its a View; since this is purely an optimization, any actual error would get thrown - // when calling DirectionConnection.connect. Hence, we can just default to false to take the non-optimized emission path - case e: Throwable => false - } - (typeEquivalent && consumer.notWaivedOrSqueezedOrExcluded && producer.notWaivedOrSqueezedOrExcluded) + // Role-tagged views must go through the slow path for role enforcement checks + val hasRole = + Connection.roleOf(consumer.base).isDefined || + Connection.roleOf(producer.base).isDefined + if (hasRole) false + else { + val typeEquivalent = + try { + BiConnect.canFirrtlConnectData( + consumer.base, + producer.base, + UnlocatableSourceInfo, + Builder.referenceUserModule + ) && consumer.base.typeEquivalent(producer.base) + } catch { + // For some reason, an error is thrown if its a View; since this is purely an optimization, any actual error would get thrown + // when calling DirectionConnection.connect. Hence, we can just default to false to take the non-optimized emission path + case e: Throwable => false + } + (typeEquivalent && consumer.notWaivedOrSqueezedOrExcluded && producer.notWaivedOrSqueezedOrExcluded) + } } } @@ -108,7 +116,70 @@ private[chisel3] object Connection { )( implicit sourceInfo: SourceInfo ): Unit = { - doConnection(cRoot, pRoot, cOp) + if (!checkRoleEnforcement(cRoot, pRoot)) { + doConnection(cRoot, pRoot, cOp) + } + } + + /** Check that role-tagged views (.asProducer/.asConsumer) are used on the correct side. + * + * @return true if a hard (non-deprecated) violation was reported, in which case the caller + * should skip the actual connection to avoid piling on per-field writability errors. + */ + private def checkRoleEnforcement[T <: Data]( + consumer: Connectable[T], + producer: Connectable[T] + )( + implicit sourceInfo: SourceInfo + ): Boolean = { + var hardViolation = false + // .asProducer must not be used on the consumer (LHS) + roleOf(consumer.base).foreach { + case (ConnectableRole.Producer, false) => + Builder.error(".asProducer cannot be used on the consumer (LHS) of a connection operator") + hardViolation = true + case (ConnectableRole.Producer, true) => + Builder.warning( + Warning( + WarningID.AsProducerDeprecated, + ".asProducer cannot be used on the consumer (LHS) of a connection operator" + ) + ) + case _ => () + } + // .asConsumer must not be used on the producer (RHS) + roleOf(producer.base).foreach { + case (ConnectableRole.Consumer, false) => + Builder.error(".asConsumer cannot be used on the producer (RHS) of a connection operator") + hardViolation = true + case (ConnectableRole.Consumer, true) => + Builder.warning( + Warning( + WarningID.AsConsumerDeprecated, + ".asConsumer cannot be used on the producer (RHS) of a connection operator" + ) + ) + case _ => () + } + hardViolation + } + + /** Detect the connectable role of a Data from its binding, if it is a role-tagged view. + * + * @return Some((role, isDeprecated)) where isDeprecated is true for the warn-only variants. + */ + private[connectable] def roleOf(data: Data): Option[(ConnectableRole, Boolean)] = { + def extract(wr: ViewWriteability): Option[(ConnectableRole, Boolean)] = wr match { + case ViewWriteability.ReadOnly(_, Some(r)) => Some((r, false)) + case ViewWriteability.ReadOnlyDeprecated(_, Some(r)) => Some((r, true)) + case _ => None + } + data.topBindingOpt match { + case Some(ViewBinding(_, wr)) => extract(wr) + case Some(avb: AggregateViewBinding) => + avb.writabilityMap.flatMap(_.values.flatMap(extract).headOption) + case _ => None + } } private def connect( diff --git a/core/src/main/scala/chisel3/experimental/dataview/package.scala b/core/src/main/scala/chisel3/experimental/dataview/package.scala index 2ae57da29a0..fc77090927e 100644 --- a/core/src/main/scala/chisel3/experimental/dataview/package.scala +++ b/core/src/main/scala/chisel3/experimental/dataview/package.scala @@ -34,7 +34,17 @@ package object dataview { val result: V = dataView.mkView(target) requireIsChiselType(result, "viewAs") - doBind(target, result, dataView, writability) + // A role-bearing writability (from .asProducer/.asConsumer) applies read-only per-field based on + // alignment: a Producer view marks aligned fields read-only, a Consumer view marks flipped fields. + val role = writability match { + case ViewWriteability.ReadOnly(_, r) => r + case ViewWriteability.ReadOnlyDeprecated(_, r) => r + case _ => None + } + val wrMap = role.map { r => + computeAlignmentWritabilityMap(result, writability, markAligned = r == ConnectableRole.Producer) + } + doBind(target, result, dataView, if (wrMap.isDefined) ViewWriteability.Default else writability, wrMap) // Setting the parent marks these Data as Views result.setAllParents(Some(ViewParent)) @@ -66,6 +76,38 @@ package object dataview { dataView: DataView[T, V], sourceInfo: SourceInfo ): V = _viewAsImpl(ViewWriteability.ReadOnly(getError)) + + private[chisel3] def viewAsProducer[V <: Data]( + getError: SourceInfo => String + )( + implicit dataproduct: DataProduct[T], + dataView: DataView[T, V], + sourceInfo: SourceInfo + ): V = _viewAsImpl(ViewWriteability.ReadOnly(getError, Some(ConnectableRole.Producer))) + + private[chisel3] def viewAsConsumer[V <: Data]( + getError: SourceInfo => String + )( + implicit dataproduct: DataProduct[T], + dataView: DataView[T, V], + sourceInfo: SourceInfo + ): V = _viewAsImpl(ViewWriteability.ReadOnly(getError, Some(ConnectableRole.Consumer))) + + private[chisel3] def viewAsProducerDeprecated[V <: Data]( + getWarning: SourceInfo => Warning + )( + implicit dataproduct: DataProduct[T], + dataView: DataView[T, V], + sourceInfo: SourceInfo + ): V = _viewAsImpl(ViewWriteability.ReadOnlyDeprecated(getWarning, Some(ConnectableRole.Producer))) + + private[chisel3] def viewAsConsumerDeprecated[V <: Data]( + getWarning: SourceInfo => Warning + )( + implicit dataproduct: DataProduct[T], + dataView: DataView[T, V], + sourceInfo: SourceInfo + ): V = _viewAsImpl(ViewWriteability.ReadOnlyDeprecated(getWarning, Some(ConnectableRole.Consumer))) } /** Provides `viewAsSupertype` for subclasses of [[Record]] */ @@ -103,10 +145,11 @@ package object dataview { // TODO should this be moved to class Aggregate / can it be unified with Aggregate.bind? private def doBind[T: DataProduct, V <: Data]( - target: T, - view: V, - dataView: DataView[T, V], - writability: ViewWriteability + target: T, + view: V, + dataView: DataView[T, V], + writability: ViewWriteability, + writabilityMapOverride: Option[Map[Data, ViewWriteability]] = None )( implicit sourceInfo: SourceInfo ): Unit = { @@ -233,16 +276,45 @@ package object dataview { } view match { - case elt: Element => view.bind(ViewBinding(elementResult(elt), writability)) + case elt: Element => + val w = writabilityMapOverride.flatMap(_.get(elt)).getOrElse(writability) + view.bind(ViewBinding(elementResult(elt), w)) case agg: Aggregate => val fullResult = elementResult ++ aggregateMappings - val aggWritability = Option.when(writability.isReadOnly)( - Map((agg: Data) -> writability) + val aggWritability = writabilityMapOverride.orElse( + Option.when(writability.isReadOnly)( + Map((agg: Data) -> writability) + ) ) agg.bind(AggregateViewBinding(fullResult, aggWritability)) } } + /** Compute a per-field writability map based on alignment. + * + * @param readOnly the ViewWriteability to apply to the marked fields + * @param markAligned if true, aligned fields get readOnly; if false, flipped fields get readOnly + */ + private def computeAlignmentWritabilityMap( + view: Data, + readOnly: ViewWriteability, + markAligned: Boolean + ): Map[Data, ViewWriteability] = { + val default = ViewWriteability.Default + // Collect all aligned members (including root, aggregates, and leaves) + val alignedSet: Set[Data] = + DataMirror.collectAlignedDeep(view) { case d => d }.toSet + // Collect all members + val allMembers: Iterable[Data] = + DataMirror.collectMembers(view) { case d => d } + // Build per-field writability: markAligned=true marks aligned fields, false marks flipped + // Note: we emit an entry for EVERY member (including Default ones) because + // AggregateViewBinding.lookupWritability relies on these entries for its parent fallback. + allMembers.map { m => + m -> (if (alignedSet.contains(m) == markAligned) readOnly else default) + }.toMap + } + // When annotating views that are not identity mappings, we need to record them for renaming // Technically, this adds any Aggregate that is not an identity mapping, // but we don't have a cheap way to check for single-target. diff --git a/core/src/main/scala/chisel3/internal/Binding.scala b/core/src/main/scala/chisel3/internal/Binding.scala index fd7c65b000c..50436e66602 100644 --- a/core/src/main/scala/chisel3/internal/Binding.scala +++ b/core/src/main/scala/chisel3/internal/Binding.scala @@ -130,6 +130,13 @@ private[chisel3] object binding { // It is a source (RHS). It may only be connected/applied to sinks. case class DontCareBinding() extends UnconstrainedBinding + /** The connectable "role" of a view created by `.asProducer` / `.asConsumer`. */ + sealed trait ConnectableRole + object ConnectableRole { + case object Producer extends ConnectableRole + case object Consumer extends ConnectableRole + } + /** Views are able to restrict writability of the target */ sealed trait ViewWriteability { @@ -142,10 +149,10 @@ private[chisel3] object binding { */ final def reportIfReadOnly[A](onPass: => A)(onFail: => A)(implicit info: SourceInfo): A = this match { case ViewWriteability.Default => onPass - case ViewWriteability.ReadOnlyDeprecated(getWarning) => + case ViewWriteability.ReadOnlyDeprecated(getWarning, _) => Builder.warning(getWarning(info)) onPass // This is just a warning so we propagate the pass value. - case ViewWriteability.ReadOnly(getError) => + case ViewWriteability.ReadOnly(getError, _) => Builder.error(getError(info)) onFail } @@ -164,7 +171,8 @@ private[chisel3] object binding { } /** Signals that will eventually become read only */ - case class ReadOnlyDeprecated(getWarning: SourceInfo => Warning) extends ViewWriteability { + case class ReadOnlyDeprecated(getWarning: SourceInfo => Warning, role: Option[ConnectableRole] = None) + extends ViewWriteability { override def combine(that: ViewWriteability): ViewWriteability = that match { case ro: ReadOnly => ro case _ => this @@ -172,7 +180,7 @@ private[chisel3] object binding { } /** Signals that are read only */ - case class ReadOnly(getError: SourceInfo => String) extends ViewWriteability { + case class ReadOnly(getError: SourceInfo => String, role: Option[ConnectableRole] = None) extends ViewWriteability { override def combine(that: ViewWriteability): ViewWriteability = this } } diff --git a/core/src/main/scala/chisel3/internal/Warning.scala b/core/src/main/scala/chisel3/internal/Warning.scala index a08dd595bc9..50699490d06 100644 --- a/core/src/main/scala/chisel3/internal/Warning.scala +++ b/core/src/main/scala/chisel3/internal/Warning.scala @@ -21,6 +21,8 @@ private[chisel3] object WarningID extends Enumeration { val ExtractFromVecSizeZero = Value(6) val BundleLiteralValueTooWide = Value(7) val AsTypeOfReadOnly = Value(8) + val AsProducerDeprecated = Value(9) + val AsConsumerDeprecated = Value(10) } import WarningID.WarningID diff --git a/docs/src/explanations/connectable.md b/docs/src/explanations/connectable.md index b889792d7c9..3fa5ea59747 100644 --- a/docs/src/explanations/connectable.md +++ b/docs/src/explanations/connectable.md @@ -4,30 +4,9 @@ title: "Connectable Operators" section: "chisel3" --- -## Table of Contents - * [Terminology](#terminology) - * [Overview](#overview) - * [Alignment: Flipped vs Aligned](#alignment-flipped-vs-aligned) - * [Input/Output](#inputoutput) - * [Connecting components with fully aligned members](#connecting-components-with-fully-aligned-members) - * [Mono-direction connection operator (`:=`)](#mono-direction-connection-operator-) - * [Connecting components with mixed alignment members](#connecting-components-with-mixed-alignment-members) - * [Bi-direction connection operator (`:<>=`)](#bi-direction-connection-operator-) - * [Port-Direction Computation versus Connection-Direction Computation](#port-direction-computation-versus-connection-direction-computation) - * [Aligned connection operator (`:<=`)](#aligned-connection-operator-) - * [Flipped connection operator (`:>=`)](#flipped-connection-operator-) - * [Coercing mono-direction connection operator (`:#=`)](#coercing-mono-direction-connection-operator-) - * [Connectable](#connectable) - * [Connecting Records](#connecting-records) - * [Defaults with waived connections](#defaults-with-waived-connections) - * [Connecting types with optional members](#connecting-types-with-optional-members) - * [Always ignore extra members (partial connection operator)](#always-ignore-errors-caused-by-extra-members-partial-connection-operator) - * [Connecting components with different widths](#connecting-components-with-different-widths) - * [Techniques for connecting structurally inequivalent Chisel types](#techniques-for-connecting-structurally-inequivalent-chisel-types) - * [Connecting different sub-types of the same super-type, with colliding names](#connecting-different-sub-types-of-the-same-super-type-with-colliding-names) - * [Connecting sub-types to super-types by waiving extra members](#connecting-sub-types-to-super-types-by-waiving-extra-members) - * [Connecting different sub-types](#connecting-different-sub-types) - * [FAQ](#faq) +import TOCInline from '@theme/TOCInline'; + + ## Terminology @@ -573,6 +552,97 @@ This generates the following Verilog, where the `special` field is not connected chisel3.docs.emitSystemVerilog(new Example15) ``` +### Enforcing producer/consumer roles (`.asProducer`/`.asConsumer`) + +A common pattern in library code is a helper that builds a bidirectional bundle, drives some of its fields internally, and hands the bundle back to the caller to finish connecting. +A `Decoupled` source is the canonical example: the library produces the `valid`/`bits` payload and expects the caller to supply only the `ready` backpressure. +But because the returned value is just an ordinary `Wire`, nothing stops a caller from accidentally driving `valid` or `bits` themselves — overwriting what the library already drove and silently breaking the design. + +`.asProducer` and `.asConsumer` let the library encode that intent into the value it returns. +They tag a component with its intended role, which does two things: + + * the fields that role is *not* allowed to drive become **read-only**, so a caller who tries to override them gets an elaboration-time error; and + * the connection operators **reject** the value if it is used on the wrong side, catching a swapped or mis-placed operand. + +Recall from [Alignment: Flipped vs Aligned](#alignment-flipped-vs-aligned) that a producer drives its aligned members and is driven on its flipped members (and vice versa for a consumer). +Accordingly: + + * `.asProducer` marks the **aligned** members read-only (a producer should never have its outputs driven), leaving flipped members writable. It may only appear on the **right-hand side**. + * `.asConsumer` marks the **flipped** members read-only (a consumer should never have its inputs driven), leaving aligned members writable. It may only appear on the **left-hand side**. + +In the example below, the `source` helper drives the data path and returns it as a producer. +The caller connects it with `:<>=`, supplying `ready` while being prevented from touching `valid`/`bits`: + +```scala mdoc:silent +import chisel3.util.{Decoupled, DecoupledIO} + +class StreamProducer extends Module { + val out = IO(Decoupled(UInt(8.W))) + + // Library helper: drives the payload internally, then hands the result back tagged + // as a producer so callers can only sink from it — never overwrite valid/bits. + def source(): DecoupledIO[UInt] = { + val w = Wire(Decoupled(UInt(8.W))) + w.valid := true.B + w.bits := 42.U + w.asProducer + } + + out :<>= source() +} +``` + +```scala mdoc:verilog +chisel3.docs.emitSystemVerilog(new StreamProducer) +``` + +Note that the library is free to drive `valid`/`bits` on the underlying wire *before* tagging it; `.asProducer` only constrains the *view* it returns, so the read-only restriction applies to the caller, not to the library code that produced the value. + +The tags catch two distinct kinds of mistake. + +**Driving a field the role does not own.** Because `.asProducer` makes aligned fields read-only, attempting to drive one is an error. +The following fails to elaborate with `Cannot connect to producer's aligned field`: + +```scala +// `valid` is aligned, so it is read-only on a producer view +Wire(Decoupled(UInt(8.W))).asProducer.valid := true.B // error! +``` + +Flipped fields remain writable, so `Wire(Decoupled(UInt(8.W))).asProducer.ready := true.B` is still allowed. +`.asConsumer` is symmetric: it makes the flipped fields (e.g. `ready`) read-only while leaving aligned fields (e.g. `valid`) writable. + +**Using a role on the wrong side.** A producer view on the left-hand side, or a consumer view on the right-hand side, is rejected regardless of which fields are touched. +Given a `consumer` and a `producer` of the same type, both of the following fail to elaborate: + +```scala +consumer.asProducer :<>= producer // error: .asProducer cannot be used on the consumer (LHS) of a connection operator +consumer :<>= producer.asConsumer // error: .asConsumer cannot be used on the producer (RHS) of a connection operator +``` + +> Note: side enforcement relies on a field being marked read-only, so a fully-aligned component (such as a bare `UInt`) carries no flipped fields for `.asConsumer` to mark, and a misuse on the right-hand side cannot be detected in that case. `.asProducer` has no such limitation because an aligned component always has aligned fields to mark. + +#### Migrating existing code + +If you want to add these annotations to an existing design without immediately breaking it, use the `.asProducerDeprecated` and `.asConsumerDeprecated` variants. +They behave identically but report a deprecation *warning* (see [Warnings](warnings)) instead of an error, so you can introduce the tags incrementally and fix the reported sites over time before switching to the hard-erroring `.asProducer`/`.asConsumer`. + +```scala mdoc:silent +class StreamProducerMigrating extends Module { + val out = IO(Decoupled(UInt(8.W))) + + // Same as `source` above, but tagging with the deprecated variant: misuse warns + // instead of erroring, so existing callers keep elaborating while you fix them. + def source(): DecoupledIO[UInt] = { + val w = Wire(Decoupled(UInt(8.W))) + w.valid := true.B + w.bits := 42.U + w.asProducerDeprecated + } + + out :<>= source() +} +``` + ## Techniques for connecting structurally inequivalent Chisel types `DataView` and `.viewAsSupertype` create a view of the component that has a different Chisel type. diff --git a/docs/src/explanations/warnings.md b/docs/src/explanations/warnings.md index 4a3644775d9..a99ca46aaec 100644 --- a/docs/src/explanations/warnings.md +++ b/docs/src/explanations/warnings.md @@ -180,3 +180,100 @@ class MyBundle extends Bundle { val x = WireInit(0.U.asTypeOf(new MyBundle)) x.bar := 123.U ``` + +### [W009] Producer view used incorrectly + +`.asProducerDeprecated` is a migration aid for `.asProducer`, which marks a `Data` as the producer +(right-hand side) of a [`Connectable`](connectable) operator. +See [Enforcing producer/consumer roles](connectable) for a full explanation of how these tags work. + +This warning most commonly arises when using a library component that returns a value tagged as a +producer: the library has already driven the producer's aligned fields (e.g. `valid` and `bits` on +a `Decoupled` source), and the caller is trying to drive one of them again. +If the assignment is accidental, remove it — the library already drives that field. +If you genuinely need to override a field the library drives, use an untagged `Wire` of the same +type directly rather than the library helper, so the producer restriction does not apply. + +More specifically, this warning fires when `.asProducerDeprecated` is used in a way that `.asProducer` +rejects as an error: + +* connecting to an *aligned* field of the producer view (aligned fields are outputs from the producer's + perspective, so they are read-only), or +* using the producer view on the consumer (left-hand) side of a connection operator. + +For example, a library helper returns a `Wire` tagged as a producer, meaning the library owns +the aligned (`valid`) field. +Trying to drive that field triggers W009: +```scala mdoc:compile-only +class MyBundle extends Bundle { + val valid = Bool() + val ready = Flipped(Bool()) +} +val w = Wire(new MyBundle) +val fromLibrary = w.asProducerDeprecated +fromLibrary.valid := true.B // valid is aligned, so it is read-only on a producer view +``` + +If the assignment is unintentional, remove it — the library already drives `valid`. +If you need to drive it yourself, use an intermediate `Wire`: +```scala mdoc:compile-only +class MyBundle extends Bundle { + val valid = Bool() + val ready = Flipped(Bool()) +} +val w = Wire(new MyBundle) +val fromLibrary = w.asProducerDeprecated +val myWire = Wire(new MyBundle) +myWire :<>= fromLibrary +// Intentional override of field driven by the library +myWire.valid := true.B +``` + +### [W010] Consumer view used incorrectly + +`.asConsumerDeprecated` is a migration aid for `.asConsumer`, which marks a `Data` as the consumer +(left-hand side) of a [`Connectable`](connectable) operator. +See [Enforcing producer/consumer roles](connectable) for a full explanation of how these tags work. + +This warning most commonly arises when using a library component that returns a value tagged as a +consumer: the library has already driven the consumer's flipped fields (i.e. the inputs it expects +from its environment, such as `ready` on a `Decoupled` sink), and the caller is trying to drive one +of them again. +If the assignment is accidental, remove it — the library already drives that field. +If you genuinely need to override a field the library drives, use an untagged `Wire` of the same +type directly rather than the library helper, so the consumer restriction does not apply. + +More specifically, this warning fires when `.asConsumerDeprecated` is used in a way that `.asConsumer` +rejects as an error: + +* connecting to a *flipped* field of the consumer view (flipped fields are inputs from the consumer's + perspective, so they are read-only), or +* using the consumer view on the producer (right-hand) side of a connection operator. + +For example, a library helper returns a `Wire` tagged as a consumer, meaning the library owns +the flipped (`ready`) field. +Trying to drive that field triggers W010: +```scala mdoc:compile-only +class MyBundle extends Bundle { + val valid = Bool() + val ready = Flipped(Bool()) +} +val w = Wire(new MyBundle) +val fromLibrary = w.asConsumerDeprecated +fromLibrary.ready := true.B // ready is flipped, so it is read-only on a consumer view +``` + +If the assignment is unintentional, remove it — the library already drives `ready`. +If you need to drive it yourself, use an intermediate `Wire`: +```scala mdoc:compile-only +class MyBundle extends Bundle { + val valid = Bool() + val ready = Flipped(Bool()) +} +val w = Wire(new MyBundle) +val fromLibrary = w.asConsumerDeprecated +val myWire = Wire(new MyBundle) +fromLibrary :<>= myWire +// Intentional override of field driven by the library +myWire.ready := true.B +``` diff --git a/release.mill b/release.mill index 363b2f0a4bf..f2d4e69b978 100644 --- a/release.mill +++ b/release.mill @@ -105,7 +105,12 @@ trait Unipublish extends ChiselCrossModule with ChiselPublishModule with Mima { ProblemFilter.exclude[DirectMissingMethodProblem]("chisel3.MemBase.forceName*"), ProblemFilter.exclude[DirectMissingMethodProblem]("chisel3.experimental.BaseModule.forceName*"), ProblemFilter.exclude[DirectMissingMethodProblem]("chisel3.VerificationStatement.forceName*"), - ProblemFilter.exclude[DirectMissingMethodProblem]("chisel3.properties.DynamicObject.forceName*") + ProblemFilter.exclude[DirectMissingMethodProblem]("chisel3.properties.DynamicObject.forceName*"), + // ViewWriteability inner types are package-private + ProblemFilter.exclude[DirectMissingMethodProblem]("chisel3.internal.binding#ViewWriteability#ReadOnly*"), + ProblemFilter.exclude[MissingTypesProblem]("chisel3.internal.binding$ViewWriteability$ReadOnly$"), + ProblemFilter.exclude[DirectMissingMethodProblem]("chisel3.internal.binding#ViewWriteability#ReadOnlyDeprecated*"), + ProblemFilter.exclude[MissingTypesProblem]("chisel3.internal.binding$ViewWriteability$ReadOnlyDeprecated$") ) def pluginVersion = v.scalaCrossToVersion(crossScalaVersion) diff --git a/src/test/scala/chiselTests/AsProducerConsumerSpec.scala b/src/test/scala/chiselTests/AsProducerConsumerSpec.scala new file mode 100644 index 00000000000..32988e35b93 --- /dev/null +++ b/src/test/scala/chiselTests/AsProducerConsumerSpec.scala @@ -0,0 +1,436 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests + +import chisel3._ +import chisel3.experimental.SourceInfo +import chisel3.probe._ +import chisel3.util.DecoupledIO +import circt.stage.ChiselStage +import org.scalactic.source.Position +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +class AsProducerConsumerSpec extends AnyFlatSpec with Matchers with LogUtils { + + class MixedBundle extends Bundle { + val data = UInt(8.W) + val valid = Bool() + val ready = Flipped(Bool()) + } + + class NestedBundle extends Bundle { + val inner = new MixedBundle + val flippedInner = Flipped(new MixedBundle) + } + + class CoercedBundle extends Bundle { + val out = Output(new MixedBundle) + val in = Input(new MixedBundle) + } + + def checkError(errMsg: String)(m: => RawModule)(implicit pos: Position): Unit = { + val e = the[ChiselException] thrownBy { + ChiselStage.elaborate(m, Array("--throw-on-first-error")) + } + e.getMessage should include(errMsg) + } + + def checkProducerAlignedError(m: => RawModule)(implicit pos: Position): Unit = + checkError("Cannot connect to producer's aligned field")(m) + + def checkConsumerFlippedError(m: => RawModule)(implicit pos: Position): Unit = + checkError("Cannot connect to consumer's flipped field")(m) + + def checkProducerOnLHSError(m: => RawModule)(implicit pos: Position): Unit = + checkError(".asProducer cannot be used on the consumer (LHS)")(m) + + def checkConsumerOnRHSError(m: => RawModule)(implicit pos: Position): Unit = + checkError(".asConsumer cannot be used on the producer (RHS)")(m) + + // Wrap `t` in a Wire, apply the given role view, and run `body` on the view. Each builder + // returns a RawModule; pass it to a check helper or ChiselStage to elaborate it. + def producerView[T <: Data](t: => T)(body: T => Unit): RawModule = new RawModule { body(Wire(t).asProducer) } + def consumerView[T <: Data](t: => T)(body: T => Unit): RawModule = new RawModule { body(Wire(t).asConsumer) } + def producerDeprecatedView[T <: Data](t: => T)(body: T => Unit): RawModule = + new RawModule { body(Wire(t).asProducerDeprecated) } + def consumerDeprecatedView[T <: Data](t: => T)(body: T => Unit): RawModule = + new RawModule { body(Wire(t).asConsumerDeprecated) } + + // Module with an aligned `out` and a Flipped `in` port (both MixedBundle), for connection tests. + def ioModule(body: (MixedBundle, MixedBundle) => Unit): RawModule = new RawModule { + val in = IO(Flipped(new MixedBundle)) + val out = IO(new MixedBundle) + body(out, in) + } + + // ======================== asProducer per-field writability ======================== + + behavior.of("asProducer") + + it should "make aligned fields of a Bundle read-only" in { + checkProducerAlignedError(producerView(new MixedBundle)(_.data := 1.U)) + checkProducerAlignedError(producerView(new MixedBundle)(_.valid := true.B)) + } + + it should "leave flipped fields of a Bundle writable" in { + ChiselStage.emitCHIRRTL(producerView(new MixedBundle)(_.ready := true.B)) + } + + it should "make a standalone UInt read-only (aligned with itself)" in { + checkProducerAlignedError(producerView(UInt(8.W))(_ := 1.U)) + } + + it should "work correctly with DecoupledIO" in { + // bits and valid are aligned (read-only as producer), ready is flipped (writable) + checkProducerAlignedError(producerView(new DecoupledIO(UInt(8.W)))(_.bits := 1.U)) + checkProducerAlignedError(producerView(new DecoupledIO(UInt(8.W)))(_.valid := true.B)) + ChiselStage.emitCHIRRTL(producerView(new DecoupledIO(UInt(8.W)))(_.ready := true.B)) + } + + it should "handle nested bundles correctly" in { + // inner.ready: flipped within aligned → flipped → writable + ChiselStage.emitCHIRRTL(producerView(new NestedBundle)(_.inner.ready := true.B)) + // flippedInner.data: aligned within flipped → flipped → writable + ChiselStage.emitCHIRRTL(producerView(new NestedBundle)(_.flippedInner.data := 1.U)) + // flippedInner.ready: flipped within flipped → aligned → read-only + checkProducerAlignedError(producerView(new NestedBundle)(_.flippedInner.ready := true.B)) + } + + it should "handle coerced (Input/Output) fields correctly" in { + // Output coerces all children to aligned → read-only + checkProducerAlignedError(producerView(new CoercedBundle)(_.out.data := 1.U)) + checkProducerAlignedError(producerView(new CoercedBundle)(_.out.ready := true.B)) + // Input coerces all children to flipped → writable + ChiselStage.emitCHIRRTL(producerView(new CoercedBundle)(_.in.data := 1.U)) + ChiselStage.emitCHIRRTL(producerView(new CoercedBundle)(_.in.ready := true.B)) + } + + it should "work correctly on RHS of :<>=" in { + ChiselStage.emitCHIRRTL(ioModule((out, in) => out :<>= in.asProducer)) + } + + it should "work correctly on RHS of :<=" in { + ChiselStage.emitCHIRRTL(ioModule((out, in) => out :<= in.asProducer)) + } + + it should "work correctly on RHS of :>=" in { + ChiselStage.emitCHIRRTL(ioModule((out, in) => out :>= in.asProducer)) + } + + it should "NOT create a view for literals" in { + ChiselStage.emitCHIRRTL(new RawModule { + val a = 123.U + assert(a.asProducer eq a) + }) + } + + it should "NOT create a view for op results" in { + ChiselStage.emitCHIRRTL(new RawModule { + val a = IO(Input(UInt(8.W))) + val x = a + 1.U + assert(x.asProducer eq x) + }) + } + + // ======================== asConsumer per-field writability ======================== + + behavior.of("asConsumer") + + it should "make flipped fields of a Bundle read-only" in { + checkConsumerFlippedError(consumerView(new MixedBundle)(_.ready := true.B)) + } + + it should "leave aligned fields of a Bundle writable" in { + ChiselStage.emitCHIRRTL(consumerView(new MixedBundle) { c => + c.data := 1.U + c.valid := true.B + }) + } + + it should "leave a standalone UInt writable (no flipped fields)" in { + ChiselStage.emitCHIRRTL(consumerView(UInt(8.W))(_ := 1.U)) + } + + it should "work correctly with DecoupledIO" in { + // ready is flipped (read-only as consumer), bits and valid are aligned (writable) + checkConsumerFlippedError(consumerView(new DecoupledIO(UInt(8.W)))(_.ready := true.B)) + ChiselStage.emitCHIRRTL(consumerView(new DecoupledIO(UInt(8.W))) { c => + c.bits := 1.U + c.valid := true.B + }) + } + + it should "handle nested bundles correctly" in { + // flippedInner.ready: flipped within flipped → aligned → writable + ChiselStage.emitCHIRRTL(consumerView(new NestedBundle)(_.flippedInner.ready := true.B)) + // inner.ready: flipped within aligned → flipped → read-only + checkConsumerFlippedError(consumerView(new NestedBundle)(_.inner.ready := true.B)) + } + + it should "work correctly on LHS of :<>=" in { + ChiselStage.emitCHIRRTL(ioModule((out, in) => out.asConsumer :<>= in)) + } + + it should "work correctly on LHS of :<=" in { + ChiselStage.emitCHIRRTL(ioModule((out, in) => out.asConsumer :<= in)) + } + + it should "work correctly on LHS of :>=" in { + ChiselStage.emitCHIRRTL(ioModule((out, in) => out.asConsumer :>= in)) + } + + it should "NOT create a view for literals" in { + ChiselStage.emitCHIRRTL(new RawModule { + val a = 123.U + assert(a.asConsumer eq a) + }) + } + + it should "NOT create a view for op results" in { + ChiselStage.emitCHIRRTL(new RawModule { + val a = IO(Input(UInt(8.W))) + val x = a + 1.U + assert(x.asConsumer eq x) + }) + } + + // ======================== Side enforcement ======================== + + behavior.of("asProducer side enforcement") + + it should "error when asProducer is used on LHS of :<>=" in { + checkProducerOnLHSError(ioModule((out, in) => out.asProducer :<>= in)) + } + + it should "error when asProducer is used on LHS of :<=" in { + checkProducerOnLHSError(ioModule((out, in) => out.asProducer :<= in)) + } + + it should "error when asProducer is used on LHS of :>=" in { + checkProducerOnLHSError(ioModule((out, in) => out.asProducer :>= in)) + } + + it should "error when asProducer is used on LHS of :#=" in { + checkProducerOnLHSError(ioModule((out, in) => out.asProducer :#= in)) + } + + behavior.of("asConsumer side enforcement") + + it should "error when asConsumer is used on RHS of :<>=" in { + checkConsumerOnRHSError(ioModule((out, in) => out :<>= in.asConsumer)) + } + + it should "error when asConsumer is used on RHS of :<=" in { + checkConsumerOnRHSError(ioModule((out, in) => out :<= in.asConsumer)) + } + + it should "error when asConsumer is used on RHS of :>=" in { + checkConsumerOnRHSError(ioModule((out, in) => out :>= in.asConsumer)) + } + + it should "error when asConsumer is used on RHS of :#=" in { + checkConsumerOnRHSError(ioModule((out, in) => out :#= in.asConsumer)) + } + + // ======================== roleOf edge cases ======================== + + behavior.of("role recovery (roleOf)") + + // Connection.roleOf recovers the Producer/Consumer role from a view's binding, but it can + // only see the role where computeAlignmentWritabilityMap actually marked a field read-only. + // A consumer view marks *flipped* fields, so a fully-aligned target (e.g. a standalone UInt + // or an all-aligned Bundle) produces an all-Default writability map and roleOf returns None. + // As a result, side enforcement does not fire for such a view used on the wrong side. The + // two tests below pin this asymmetry: the aligned *producer* view (which marks the aligned + // field read-only) is caught, but the aligned *consumer* view is not. If roleOf is taught to + // track the role independently of per-field writability, the second test should tighten to a + // checkConsumerOnRHSError. + + it should "catch an all-aligned asProducer used on the LHS (role recovered from the read-only field)" in { + checkProducerOnLHSError(new RawModule { + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + out.asProducer :<>= in + }) + } + + it should "(known limitation) NOT catch an all-aligned asConsumer used on the RHS" in { + // A standalone UInt is fully aligned, so asConsumer marks nothing read-only and the + // Consumer role is lost. The misuse on the RHS therefore elaborates without error. + ChiselStage.emitCHIRRTL(new RawModule { + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + out :<>= in.asConsumer + }) + } + + // ======================== Combined usage ======================== + + behavior.of("asProducer and asConsumer together") + + it should "work when both are used correctly" in { + ChiselStage.emitCHIRRTL(ioModule((out, in) => out.asConsumer :<>= in.asProducer)) + } + + // ======================== Deprecated variants ======================== + + def checkHasWarning(warnMsg: String)(m: => RawModule)(implicit pos: Position): Unit = { + val (log, _) = grabLog(ChiselStage.emitCHIRRTL(m)) + log should include(warnMsg) + } + + def checkNoWarning(warnMsg: String)(m: => RawModule)(implicit pos: Position): Unit = { + val (log, _) = grabLog(ChiselStage.emitCHIRRTL(m)) + (log should not).include(warnMsg) + } + + // The per-operator side-enforcement matrix is covered by the hard variants above; the + // deprecated variants only spot-check :<>= since they share the same enforcement path. + behavior.of("asProducerDeprecated") + + it should "warn (not error) when connecting to aligned fields" in { + checkHasWarning("Cannot connect to producer's aligned field")( + producerDeprecatedView(new MixedBundle)(_.data := 1.U) + ) + } + + it should "leave flipped fields writable without warning" in { + checkNoWarning("producer")(producerDeprecatedView(new MixedBundle)(_.ready := true.B)) + } + + it should "warn (not error) when used on LHS" in { + checkHasWarning(".asProducer cannot be used on the consumer (LHS)")( + ioModule((out, in) => out.asProducerDeprecated :<>= in) + ) + } + + it should "work correctly on RHS without warning" in { + checkNoWarning("producer")(ioModule((out, in) => out :<>= in.asProducerDeprecated)) + } + + behavior.of("asConsumerDeprecated") + + it should "warn (not error) when connecting to flipped fields" in { + checkHasWarning("Cannot connect to consumer's flipped field")( + consumerDeprecatedView(new MixedBundle)(_.ready := true.B) + ) + } + + it should "leave aligned fields writable without warning" in { + checkNoWarning("consumer")(consumerDeprecatedView(new MixedBundle)(_.data := 1.U)) + } + + it should "warn (not error) when used on RHS" in { + checkHasWarning(".asConsumer cannot be used on the producer (RHS)")( + ioModule((out, in) => out :<>= in.asConsumerDeprecated) + ) + } + + it should "work correctly on LHS without warning" in { + checkNoWarning("consumer")(ioModule((out, in) => out.asConsumerDeprecated :<>= in)) + } + + // ======================== Vec ======================== + + behavior.of("asProducer/asConsumer with Vec") + + it should "make aligned Vec[Bundle] elements read-only as producer" in { + // data is aligned → read-only as producer + checkProducerAlignedError(producerView(Vec(2, new MixedBundle))(v => v(0).data := 1.U)) + // ready is flipped → writable as producer + ChiselStage.emitCHIRRTL(producerView(Vec(2, new MixedBundle))(v => v(0).ready := true.B)) + } + + it should "make flipped Vec[Bundle] elements read-only as consumer" in { + // ready is flipped → read-only as consumer + checkConsumerFlippedError(consumerView(Vec(2, new MixedBundle))(v => v(0).ready := true.B)) + // data and valid are aligned → writable as consumer + ChiselStage.emitCHIRRTL(consumerView(Vec(2, new MixedBundle)) { v => + v(0).data := 1.U + v(0).valid := true.B + }) + } + + it should "preserve per-leaf alignment for a Flipped(Vec[Bundle]) viewed as producer" in { + // Observed behavior: Flipped on a Wire of an aggregate does NOT invert the per-leaf + // alignment of the resulting hardware (a Wire is its own reference; the outer Flipped on + // the wire's type is coerced away), so the alignment matches the plain Vec case. + checkProducerAlignedError(producerView(Flipped(Vec(2, new MixedBundle)))(v => v(0).data := 1.U)) + ChiselStage.emitCHIRRTL(producerView(Flipped(Vec(2, new MixedBundle)))(v => v(0).ready := true.B)) + } + + it should "make all elements of a plain Vec[UInt] read-only as producer" in { + checkProducerAlignedError(producerView(Vec(2, UInt(8.W)))(v => v(0) := 1.U)) + } + + // ======================== Probe ======================== + + behavior.of("asProducer/asConsumer with Probe") + + class ProbeBundle extends Bundle { + val data = UInt(8.W) + val p = Probe(Bool()) + } + + it should "view a probe-containing bundle as producer and keep aligned fields read-only" in { + checkProducerAlignedError(new RawModule { + val w = Wire(new ProbeBundle) + // Make the underlying wire legal by defining the probe. + define(w.p, ProbeValue(WireInit(false.B))) + w.asProducer.data := 1.U // aligned → read-only as producer + }) + } + + // ======================== DontCare ======================== + + behavior.of("asProducer/asConsumer with DontCare") + + it should "allow assigning DontCare to a writable (flipped) producer-view field" in { + ChiselStage.emitCHIRRTL(producerView(new MixedBundle)(_.ready := DontCare)) // flipped → writable + } + + it should "still hard-error when assigning DontCare to an aligned producer-view field" in { + checkProducerAlignedError(producerView(new MixedBundle)(_.data := DontCare)) // aligned → read-only + } + + it should "allow plain assignment of DontCare to an unviewed wire" in { + ChiselStage.emitCHIRRTL(new RawModule { + val out = Wire(new MixedBundle) + out := DontCare + }) + } + + // ======================== CHIRRTL equivalence ======================== + + behavior.of("CHIRRTL equivalence") + + it should "connect the same leaf fields for out.asConsumer :<>= in.asProducer as out :<>= in" in { + // The plain form emits a single bulk connect ("connect out, in"); the view form expands + // it into the equivalent explicit per-leaf connects ("connect out.data, in.data", etc., + // with the flipped `ready` reversed). Both are semantically identical; the only difference + // is structural (bulk vs. inlined per-leaf connects). + val plain = ChiselStage.emitCHIRRTL(ioModule((out, in) => out :<>= in)) + val viewed = ChiselStage.emitCHIRRTL(ioModule((out, in) => out.asConsumer :<>= in.asProducer)) + plain should include("connect out, in") + viewed should include("connect out.data, in.data") + } + + // ======================== Composition with .readOnly ======================== + + behavior.of("composition with .readOnly (regression for stacked writability)") + + it should "hard-error (not warn) when writing through readOnly.asProducerDeprecated" in { + // The underlying .readOnly is a hard read-only; stacking the deprecated producer view + // must NOT downgrade it to a warning. + checkError("Cannot connect to read-only value")(new RawModule { + Wire(new MixedBundle).readOnly.asProducerDeprecated.data := 1.U + }) + } + + it should "hard-error (not warn) when writing a flipped field through readOnly.asConsumerDeprecated" in { + checkError("Cannot connect to read-only value")(new RawModule { + Wire(new MixedBundle).readOnly.asConsumerDeprecated.ready := true.B + }) + } +}